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

add query param to search registry records. #1861

Merged
merged 12 commits into from
Jan 22, 2025
Merged

Conversation

Faakhir30
Copy link
Contributor

@Faakhir30 Faakhir30 commented Jan 19, 2025

closes #1859

Added query param to filter out records, and returning only records with key that startswith q param on listing registry service.
Created a tmp registery with filtered records, serialize registry, return :)

  • Add http examples
  • Add/Update tests
  • Update docs

📚 Documentation preview 📚: https://plonerestapi--1861.org.readthedocs.build/

@mister-roboto
Copy link

@Faakhir30 thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@Faakhir30
Copy link
Contributor Author

@jenkins-plone-org please run jobs

@Faakhir30 Faakhir30 marked this pull request as ready for review January 19, 2025 23:02
@stevepiercy
Copy link
Contributor

Anyone know why Jenkins CI for 6.0/3.11 and 6.1/3.12 do not show up on the build history?

https://jenkins.plone.org/view/Core/builds

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Minor improvements to docs and news.

docs/source/endpoints/registry.md Outdated Show resolved Hide resolved
docs/source/endpoints/registry.md Outdated Show resolved Hide resolved
news/1861.feature Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Contributor

Direct link to docs preview:

https://plonerestapi--1861.org.readthedocs.build/en/1861/endpoints/registry.html#filtering-registry-records

@Faakhir30 thank you for your work on this!

@stevepiercy stevepiercy requested review from mauritsvanrees, erral and a team January 20, 2025 02:38
Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

I'll remove the extra blank line.

docs/source/endpoints/registry.md Outdated Show resolved Hide resolved
Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Docs and news LGTM, and it's all green. Let's get a technical review, too. Thank you! This is really nice. I think @gforcada would be interested in this.

@stevepiercy stevepiercy requested a review from gforcada January 20, 2025 11:04
@davisagli
Copy link
Member

Anyone know why Jenkins CI for 6.0/3.11 and 6.1/3.12 do not show up on the build history?

@stevepiercy We have Jenkins configured to only run jobs for the oldest and newest version of Python supported by each version of Plone, to save some CPU cycles/carbon (so currently Python 3.9 and 3.13 for Plone 6.0, and Python 3.10 and 3.13 for Plone 6.1). I updated the required jobs in the branch protection settings for this repository so it should no longer show the others.

I'll review this in the next day or two. Today was a holiday so I'm not spending much time at my computer.

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

Nice work. A couple changes please:

news/1861.feature Outdated Show resolved Hide resolved
src/plone/restapi/services/registry/get.py Outdated Show resolved Hide resolved
@davisagli davisagli merged commit 88f2bc9 into main Jan 22, 2025
20 of 24 checks passed
@davisagli davisagli deleted the query_registry_listing branch January 22, 2025 03:01
@davisagli
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add query param in listing registry endpoint
4 participants