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

feat(api): get inactive copyright details for a file #2485

Closed
wants to merge 1 commit into from
Closed

feat(api): get inactive copyright details for a file #2485

wants to merge 1 commit into from

Conversation

soham4abc
Copy link
Contributor

@soham4abc soham4abc commented Jun 20, 2023 •
edited

Description

Inactive copyright info for file is exposed through /file/upload/{UploadId}/item/{ItemId}/copyrights/inactive endpoint

Screenshots

Screenshot from 2023-06-20 20-50-25

How to test

Send a GET request to /file/upload/{UploadId}/item/{ItemId}/copyrights/inactive endpoint to see the result.

closes #2468

@soham4abc soham4abc mentioned this pull request Jun 22, 2023
Closed
@GMishx GMishx added needs code review needs test GSOC-23 Label to tag pull request which are part of the GSOC 2023 labels Jun 22, 2023
@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch from 9da6895 to 50e07a5 Compare June 23, 2023 05:25
@soham4abc soham4abc mentioned this pull request Jun 24, 2023
Open
@github-actions
Copy link

This pull request has conflicts, please rebase with master to resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the has merge conflicts PR to be rebased label Jun 29, 2023
@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch from 50e07a5 to b6a76a6 Compare June 29, 2023 05:45
@github-actions github-actions bot removed the has merge conflicts PR to be rebased label Jun 29, 2023
@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch from b6a76a6 to d472fb5 Compare June 29, 2023 05:50
@github-actions github-actions bot added the has merge conflicts PR to be rebased label Jun 29, 2023
@github-actions
Copy link

This pull request has conflicts, please rebase with master to resolve those before we can evaluate the pull request.

@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch from d472fb5 to a7e713d Compare June 29, 2023 05:53
@github-actions github-actions bot removed the has merge conflicts PR to be rebased label Jun 29, 2023
@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch 2 times, most recently from 15665aa to bc9c114 Compare June 29, 2023 06:26
GMishx
GMishx requested changes Jun 29, 2023
View reviewed changes
src/copyright/ui/ajax-copyright-hist.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/CopyrightController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/CopyrightController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/CopyrightController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/CopyrightController.php Outdated Show resolved Hide resolved
src/www/ui/api/Controllers/CopyrightController.php Show resolved Hide resolved
src/www/ui/api/Controllers/CopyrightController.php Outdated Show resolved Hide resolved
src/www/ui/api/documentation/openapi.yaml Outdated
@@ -1728,6 +1728,56 @@ paths:
default:
$ref: '#/components/responses/defaultResponse'

/file/upload/{id}/item/{ItemId}/copyrights/inactive:
Copy link
Member

Choose a reason for hiding this comment

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

Please be consistent with other endpoints

Suggested change
/file/upload/{id}/item/{ItemId}/copyrights/inactive:
/uploads/{id}/items/{itemId}/copyrights:

Copy link
Member

Choose a reason for hiding this comment

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

Would suggest to use a query parameter like filter with enum active, inactive rather than creating separate endpoints to get active and inactive copyrights.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes sir, I will be implementing this and updating one of the PRs for this operation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would suggest to use a query parameter like filter with enum active, inactive rather than creating separate endpoints to get active and inactive copyrights.

This PR's feature is implemented in #2475 using query parameters.

@GMishx please do have a look when possible.

Copy link
Member

Choose a reason for hiding this comment

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

So can I close this in favor of #2475 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure sir!

@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch 2 times, most recently from 86788d9 to 162746b Compare June 30, 2023 05:26
@github-actions github-actions bot added the has merge conflicts PR to be rebased label Jun 30, 2023
@github-actions
Copy link

This pull request has conflicts, please rebase with master to resolve those before we can evaluate the pull request.

@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch from 162746b to c26e112 Compare June 30, 2023 05:27
@soham4abc soham4abc force-pushed the copyrights/getInactiveDetails branch from c26e112 to 8068b20 Compare June 30, 2023 05:35
@GMishx
Copy link
Member

GMishx commented Jul 1, 2023

Closing in favor of #2475

@GMishx GMishx closed this Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GMishx GMishx GMishx requested changes

Assignees
No one assigned
Labels
GSOC-23 Label to tag pull request which are part of the GSOC 2023 has merge conflicts PR to be rebased needs code review needs test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Copyright File-view
2 participants
@soham4abc @GMishx