-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API - List Applications OrderBy #569
Conversation
02ea06c
to
1730887
Compare
@shaangill025 i will try to update getNext app to use the oldest Host application, thanks for flagging it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -167,6 +169,17 @@ def get_applications(): | |||
name: Account-Id | |||
required: true | |||
type: string | |||
- in: query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the right endpoint to make the changes? I thought examiner dashboard is using search_applications. If examiner dashboard is using this they need to pass the sort order as Asc explicitly. Client UIs which uses this should still see the latest applications first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dimak1 shared this, both endpoints being used:
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
Signed-off-by: Shaanjot Gill <[email protected]>
1730887
to
854b13e
Compare
@kris-daxiom All the requested changes are now included. |
Signed-off-by: Shaanjot Gill <[email protected]>
|
Temporary Url for review: https://strr-hosts-dev--pr-569-5z4s8f21.web.app |
@shaangill025 approve the label update. There is one unresolved comment from Leshmi. |
That initial comment has been resolved by implementing sorting support for |
Issue:
Description of changes:
Addresses @dimak1 comment:
One item is missing from the list - the logic for "get next" application that we did not finalized.
Currently, if examiner clicks 'Examine` tab or Approve/Refuse buttons, they are navigated to next application details page. The next application by default is first Host application under Full Review returned by API.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the BC Registry and Digital Services BSD 3-Clause License