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.

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

Use collection_id path parameter Items Transactions endpoints #425

Merged
merged 8 commits into from
Aug 2, 2022

Conversation

duckontheweb
Copy link
Contributor

@duckontheweb duckontheweb commented Jul 20, 2022

Related Issue(s):

Description:

PR Checklist:

  • Code is formatted and linted (run pre-commit run --all-files)
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable, and docs build successfully (run make docs)
  • Changes are added to the CHANGELOG.

@duckontheweb
Copy link
Contributor Author

I think I'm fairly close to fixing the sqlalchemy backend and tests. I'll push some changes shortly to get the CI passing again.

@duckontheweb duckontheweb force-pushed the change/406-create-item-collection-id branch from 160f831 to f4b9b89 Compare July 21, 2022 20:37
duckontheweb added a commit that referenced this pull request Jul 21, 2022
@duckontheweb
Copy link
Contributor Author

I think I'm fairly close to fixing the sqlalchemy backend and tests. I'll push some changes shortly to get the CI passing again.

This has been fixed and this is ready for review.

duckontheweb added a commit that referenced this pull request Aug 1, 2022
@duckontheweb duckontheweb force-pushed the change/406-create-item-collection-id branch from 8529d07 to ba29553 Compare August 1, 2022 19:19
@geospatial-jeff
Copy link
Collaborator

From the transactions spec (https://github.com/radiantearth/stac-api-spec/blob/master/ogcapi-features/extensions/transaction/README.md):

  • POST requests shall populate the collection field in the item from the URI.
  • PUT requests should return a 400 status code if id or collection fields are different from those in the URI.

@duckontheweb
Copy link
Contributor Author

From the transactions spec (https://github.com/radiantearth/stac-api-spec/blob/master/ogcapi-features/extensions/transaction/README.md):

  • POST requests shall populate the collection field in the item from the URI.
  • PUT requests should return a 400 status code if id or collection fields are different from those in the URI.

This is the current behavior for this PR, except it returns a 409 status code if the id or collection fields are different from those in the URI. I can update the response to match this.

@duckontheweb duckontheweb force-pushed the change/406-create-item-collection-id branch from ba29553 to 94c2259 Compare August 2, 2022 01:18
@duckontheweb duckontheweb removed the request for review from moradology August 2, 2022 01:18
@duckontheweb
Copy link
Contributor Author

@geospatial-jeff I fixed the response code, so this is ready for final review and/or merge.

@philvarner Let me know if you have any feedback on this.

Copy link
Collaborator

@geospatial-jeff geospatial-jeff left a comment

Choose a reason for hiding this comment

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

LGTM!

@geospatial-jeff geospatial-jeff merged commit d2fcf13 into master Aug 2, 2022
@geospatial-jeff geospatial-jeff deleted the change/406-create-item-collection-id branch August 2, 2022 17:50
geospatial-jeff added a commit that referenced this pull request Aug 2, 2022
* Add collection_id path parameter and check against Item collection property

* Fix unformatted f-strings

* Fix Item PUT endpoint per #385

* Update API tests to use new PUT paths

* Make equivalent changes to sqlalchemy backend

* Add CHANGELOG entry for #425

* Fix failing tests from previous merge

* Return 400 for Item id or collection conflicts

* Add failing tests

* Return added Item/Collection from Transactions endpoints

* Format changes

* Add CHANGES entry for #424

* Get Transactions Item response collection id from path parameter

* remove duplicate test cases

Co-authored-by: Jeff Albrecht <geospatialjeff@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment