-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Improvement] Custom Reports tree endpoint #665
base: 1.x
Are you sure you want to change the base?
Conversation
…ting-endpoint' into 660-task-custom-reports-tree-listing-endpoint # Conflicts: # src/CustomReport/Repository/CustomReportRepository.php
…ting-endpoint' into 660-task-custom-reports-tree-listing-endpoint
* @throws InvalidQueryTypeException | ||
*/ | ||
#[Route('/custom-reports/config/tree', name: 'pimcore_studio_api_report', methods: ['GET'])] | ||
#[IsGranted(UserPermissions::TAGS_SEARCH->value)] |
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 correct permission?
* @throws InvalidQueryTypeException | ||
*/ | ||
#[Route('/custom-reports/tree', name: 'pimcore_studio_api_report', methods: ['GET'])] | ||
#[IsGranted(UserPermissions::TAGS_SEARCH->value)] |
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 correct permission?
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.
Obviously not, had some questions about how permissions are working and wanted to add more endpoints first. But thanks for reminding me.
*/ | ||
#[Schema( | ||
title: 'Custom Report', | ||
type: 'object' |
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.
Please add the required fields here. Like every field that is not nullable is required basically
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.
What does required in case of strings mean? Null or empty?
*/ | ||
#[Schema( | ||
title: 'Custom Report', | ||
type: 'object' |
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.
Same here
…ting-endpoint' into 660-task-custom-reports-tree-listing-endpoint
Thanks for reviewing, in general I would prefer reviews after a pull request is done and I "request" an review. Should we, for the future, use the draft option for that? |
…ting-endpoint' into 660-task-custom-reports-tree-listing-endpoint
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
Changes in this pull request
Resolves #660
Resolves #661
Additional info
Added 2 endpoints:
1.) Get reports to display in a tree, that is used to actually display the reports
2.) Get reports to display in a tree, that is used for configuring the reports