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

feat: add snap_name to Rating and Vote messages #153

Merged
merged 6 commits into from
Dec 12, 2024

Conversation

d-loose
Copy link
Member

@d-loose d-loose commented Dec 11, 2024

This adds snap_name to the Rating message which is part of GetRatingResponse and GetChartResponse. This spares us from having to resolve snap_ids into snap_names in the app-center when requesting top charts from the server.

To simplify the transition from snap_ids to snap_names in the future, this also adds a snap_name field to the Vote message.

UDENG-5628

@d-loose d-loose force-pushed the snap-names-in-charts branch from 7acd796 to 2a2b964 Compare December 12, 2024 15:23
@d-loose d-loose changed the title feat: add snap_name to Rating message feat: add snap_name to Rating and Vote messages Dec 12, 2024
@d-loose d-loose marked this pull request as ready for review December 12, 2024 15:33
@d-loose
Copy link
Member Author

d-loose commented Dec 12, 2024

@sminez I've moved all of the snap name resolving logic to the grpc layer now. I'm still using try_join_all to fetch the snap names for votes or chart data in bulk - let me know if you had something different in mind.

@d-loose d-loose requested a review from sminez December 12, 2024 15:36
src/grpc/app.rs Outdated Show resolved Hide resolved
src/ratings/rating.rs Outdated Show resolved Hide resolved
src/grpc/charts.rs Outdated Show resolved Hide resolved
src/grpc/user.rs Show resolved Hide resolved
@d-loose d-loose force-pushed the snap-names-in-charts branch from 64c8fb6 to 035f6c7 Compare December 12, 2024 16:19
@d-loose d-loose merged commit c5a3418 into ubuntu:main Dec 12, 2024
4 checks passed
@d-loose d-loose deleted the snap-names-in-charts branch December 12, 2024 16:31
d-loose added a commit to ubuntu/app-center that referenced this pull request Dec 17, 2024
Follow-up to ubuntu/app-center-ratings#153

Uses the `snap_name` field added to `Rating` and `Vote` server-side,
which eliminates the need to resolve `snap_id`s into `snap_name`s in the
app-center.

Since this needs a regeneration of the protobuf files, I've also removed
the `listMyVotes` methods following
ubuntu/app-center-ratings#151.

UDENG-5628
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.

2 participants