add [properties-be] - properties service handling system #456
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Modified get_entity_type_from_entity_property to exclude system properties by joining property_definitions and filtering on is_system = FALSE - System properties now return 404 Not Found when deletion is attempted - System property values can still be modified via set endpoint - Added documentation explaining this behavior
whutchinson98
requested changes
Dec 5, 2025
rust/cloud-storage/properties_service/src/api/properties/definitions/delete.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties_service/src/api/properties/entities/delete_property.rs
Show resolved
Hide resolved
rust/cloud-storage/properties_service/src/api/properties/options/create.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties_service/src/api/properties/options/create.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties_service/src/api/properties/options/create.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties_service/src/api/properties/options/delete.rs
Outdated
Show resolved
Hide resolved
Fields already captured by #[tracing::instrument] are now removed from individual log statements to avoid duplication in traces.
Contributor
So system properties like created_at and updated_at can be edited manually by the user? That doesn't make much sense to me. +1 to hutch's concerns about keeping these in sync with the document metadata (and search for that matter). I don't think metadata that exists elsewhere on an item (name, created_at, updated_at) should also be a property for these reasons |
404 responses are normal client errors and don't need to be logged. Simplify ok_or_else closures to plain ok_or calls.
… logging Add err attribute to all #[tracing::instrument] macros to automatically log errors on function return. Remove redundant manual tracing calls before error returns.
Replace 'Database error: {0}' with generic 'An internal error occurred'
to avoid leaking internal database details to API clients. Errors are
still logged server-side via instrument(err).
- Use 'An internal error occurred' for all internal/database errors
- Use '[Entity] not found' pattern for not-found errors (capitalized)
- Use 'Access denied' for permission errors (no details leaked)
- Use '{0}' passthrough for nested errors and validation messages
- Consistent capitalization across all error messages
Contributor
Author
|
metadata are derived fields (e.g. owner, created_at, updated_at). these are not stored in properties db |
whutchinson98
approved these changes
Dec 5, 2025
danielkweon
deleted the
daniel/m-5328--add-properties-backed-properties-service-handling-system
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add [properties-be] - properties service handling system
This branch adds support for system properties - properties that are managed by the system rather than users/organizations.
frontend works as is - changes are backwards compatible. but frontend changes to handle system properties incoming as well
