Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[API] Sites endpoint #7771

Merged
merged 11 commits into from
Mar 28, 2023
Merged

[API] Sites endpoint #7771

merged 11 commits into from
Mar 28, 2023

Conversation

xlecours
Copy link
Contributor

@xlecours xlecours commented Nov 3, 2021

API 0.0.4 dev
Returns the list of sites for an /api/v0.0.0.4-dev/sites GET request.

This is a rebased version of #7466

@xlecours xlecours mentioned this pull request Nov 3, 2021
Closed
@xlecours xlecours changed the title Address comments [API] Sites endpoint Nov 3, 2021
christinerogers
christinerogers requested changes Nov 3, 2021
View reviewed changes
Copy link
Contributor

@christinerogers christinerogers left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Xavier, I've reviewed the alias/mri_alias part and committed my suggested changes -- thanks.
Any other parts of this PR I should look at in particular?

@xlecours
Copy link
Contributor Author

xlecours commented Nov 4, 2021

@christinerogers Thanks for the corrections. Since you mentioned that some clients required this endpoint, can you tell if the current response structure and content is satisfying?

@christinerogers
Copy link
Contributor

christinerogers commented Nov 9, 2021 •
edited
Loading

@christinerogers Thanks for the corrections. Since you mentioned that some clients required this endpoint, can you tell if the current response structure and content is satisfying?

Hi Xavier, thanks - given the Sites call returns the same info/response structure, I give my 👍 - the wizard expects a structure w Name field where it's used, if I read it right.
cc @jesscall and @maltheism in case she can take a look.

@maltheism maltheism added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Oct 31, 2022
maltheism
maltheism previously requested changes Oct 31, 2022
View reviewed changes
Copy link
Member

@maltheism maltheism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflict with Changelog

@laemtl laemtl added this to the 25.0.0 milestone Feb 28, 2023
@laemtl laemtl added the Critical to release PR or issue is key for the release to which it has been assigned label Mar 14, 2023
@laemtl
Copy link
Contributor

laemtl commented Mar 14, 2023

@xlecours I need that feature for the Wizard. Any chance that can be part of the next release? Let me know if I can help.

@xlecours xlecours force-pushed the 2021-06-07-api-sites-endpoint branch from 05fd582 to f2bff56 Compare March 21, 2023 14:42
@xlecours xlecours dismissed maltheism’s stale review March 21, 2023 15:07

conflicts fixed

@xlecours xlecours added Category: Feature PR or issue that aims to introduce a new feature Category: Documentation PR or issue that aims to improve the documentation (test plans, wiki, comments...) and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Mar 21, 2023
@ridz1208 ridz1208 assigned christinerogers and unassigned xlecours Mar 21, 2023
driusan
driusan reviewed Mar 22, 2023
View reviewed changes
modules/api/docs/LorisRESTAPI_v0.0.4-dev.md
GET /sites
```

Will return a list of sites in this Loris instance. There is no corresponding PUT or PATCH
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended to be all sites in the LORIS instance or all sites to which the current user has access?

@driusan driusan merged commit 0048ec7 into aces:main Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driusan driusan driusan left review comments

@jesscall jesscall jesscall left review comments

@christinerogers christinerogers christinerogers approved these changes

@maltheism maltheism maltheism left review comments

Assignees

@christinerogers christinerogers

Labels
Category: Documentation PR or issue that aims to improve the documentation (test plans, wiki, comments...) Category: Feature PR or issue that aims to introduce a new feature Critical to release PR or issue is key for the release to which it has been assigned
Projects
None yet
Milestone
25.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@xlecours @christinerogers @laemtl @driusan @maltheism @jesscall