-
Notifications
You must be signed in to change notification settings - Fork 114
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
Generate pretty report identifiers #3334
Conversation
WalkthroughThe changes primarily focus on enhancing the report generation process in the admin server and ensuring the Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (17)
- proto/gen/rill/admin/v1/admin.swagger.yaml
- proto/gen/rill/admin/v1/api.pb.go
- proto/gen/rill/admin/v1/api.pb.gw.go
- proto/gen/rill/admin/v1/api.pb.validate.go
- web-admin/src/client/gen/admin-service/admin-service.ts
- web-admin/src/client/gen/index.schemas.ts
- web-common/src/proto/gen/rill/admin/v1/api_pb.ts
- web-common/src/proto/gen/rill/admin/v1/internal_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/api_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/catalog_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/connectors_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/export_format_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/queries_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/resources_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/schema_pb.ts
- web-common/src/proto/gen/rill/runtime/v1/time_grain_pb.ts
- web-common/src/proto/gen/rill/ui/v1/dashboard_pb.ts
Files selected for processing (2)
- admin/server/reports.go (3 hunks)
- proto/rill/admin/v1/api.proto (3 hunks)
Additional comments: 6
admin/server/reports.go (3)
9-9: The import of the
regexp
package is new. Ensure that it is used in the code and not an unused import.98-101: The
generateReportName
function is being used to generate a unique name for the report. This is a change from the previous implementation where a UUID was directly used as the report name. Ensure that this change does not break any dependencies that might be expecting the report name to be a UUID.480-516: The
generateReportName
andrandomReportName
functions have been added. These functions generate a user-friendly, unique report name based on the title. ThegenerateReportName
function checks if the generated name is already in use and retries up to 5 times with different names. If all names are taken, a UUID is used as a fallback. This is a good approach to ensure uniqueness of the report name while also making it user-friendly.proto/rill/admin/v1/api.proto (3)
1019-1019: The
options
field inCreateReportRequest
is now required. This change enforces that theoptions
field must be provided when creating a report. Ensure that all calls to this function throughout the codebase have been updated to match the new requirement.1030-1030: The
options
field inEditReportRequest
is now required. This change enforces that theoptions
field must be provided when editing a report. Ensure that all calls to this function throughout the codebase have been updated to match the new requirement.1064-1064: The
options
field inGenerateReportYAMLRequest
is now required. This change enforces that theoptions
field must be provided when generating a report YAML. Ensure that all calls to this function throughout the codebase have been updated to match the new requirement.
Example:
My report!
becomesmy-report-5b3f7e1a
Summary by CodeRabbit