Skip to content

Add is_owner and is_default to the Account message as read-only fields#114

Closed
Rendez wants to merge 1 commit into
mainfrom
chore/merino/CF-980
Closed

Add is_owner and is_default to the Account message as read-only fields#114
Rendez wants to merge 1 commit into
mainfrom
chore/merino/CF-980

Conversation

@Rendez
Copy link
Copy Markdown
Contributor

@Rendez Rendez commented Jun 20, 2025

Ref: https://qdrant.atlassian.net/browse/CF-980

Instead of checking for owner_email against user.email all the time, which is sort of cumbersome, let’s add a is_owner bool, much like is_default is there for the same purpose.

@Rendez Rendez requested a review from a team as a code owner June 20, 2025 12:29
@Rendez Rendez added enhancement New feature or request chore labels Jun 20, 2025
@github-actions
Copy link
Copy Markdown

ghost commented Jun 20, 2025

The latest Buf updates on your PR. Results from workflow Pull Request / linting (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 20, 2025, 12:30 PM

Comment on lines +421 to +422
// This field is read-only and is set by the server based on the authenticated user.
// It indicates whether the authenticated user is the owner of the account.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please switch order of lines (on all places we mention read-only / required) not to start with...

Comment on lines +424 to +425
// This field is read-only and is set by the server based on the authenticated user.
// It indicates whether the account is the default account for the authenticated user.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please switch order of lines (on all places we mention read-only / required) not to start with...

@Rendez
Copy link
Copy Markdown
Contributor Author

Rendez commented Jun 20, 2025

After discussing with @areina , I am dropping my PR, and sadly we already added is_owner to AccountMember, but that's alright, we'll try not to add computed/dynamic fields to Proto(s).

In the end, it was all about an agreement on how to massage data on our end.

@Rendez Rendez closed this Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants