Doesn't recognise own reviews

Bug #1564209 reported by Robert Ancell on 2016-03-31
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Ratings and Reviews server
Undecided
Unassigned
Release Notes for Ubuntu
Undecided
Unassigned
gnome-software (Ubuntu)
Medium
Unassigned

Bug Description

A review you have posted is not recognised as your own review. It should allow you to edit an existing review.

Related branches

Robert Ancell (robert-ancell) wrote :

This requires us to know the username associated with the Ubuntu One account. This is not the same as the consumer_key from OAuth.

Robert Ancell (robert-ancell) wrote :

Once you can match the username it's just a matter of:
gs_review_set_flags (review, GS_REVIEW_FLAG_SELF)

And GNOME Software will handle the UI for everything.

dobey (dobey) wrote :

For click packages we compare consumer_key == reviewer_username. It seems the old API however returns LP usernames for some people, which of course won't match the consumer_key.

If we could change the old API on the server to also always return consumer_key in the reviewer_username field, then the gnome-software plug-in could use this same tactic. The only major concern here is if it will break software-center for 14.04 users.

Martin Albisetti (beuno) wrote :

Most SSO users won't have a username as they are only generated by Launchpad.

Michael Nelson (michael.nelson) wrote :

@fgallina or @kelvin: Can you check the background for these decisions. I'm not 100% but from memory RnR used to use the SSO (well, LP) username when it has one for the user (as SSO users don't always have a username, it depends whether they have an LP account), so I don't know that we can provide a username consistently. But in Robert's case I'd imagine it's for a user who certainly does have a username (his own account?)

The only consistent options I can imagine are:
 1) always return the consumer key - @Robert - can you use that instead to identify (sorry, I don't know the background), and will this break anything (doubt it, if they're not broken already - if it does currently return either)
 2) add a separate consumer_key which always has the consumer key.

Rodney is keen for (1) so that it's consistent with the click rnr api:

21:47 < noodles> dobey: could you use a different key from the result, I mean if consumer_key was added separately?
21:48 < dobey> noodles: well i think we should match what we're doing for click; and i think software-center might be doing this comparison already anyway
21:57 < dobey> noodles: so, imo, always returning the consumer_key value for everyone in reviewer_username there, would be best
21:59 < noodles> dobey: Right, but that's exactly what Robert *doesn't* want, unless he could also identify with the consumer key?
21:59 < dobey> noodles: he can identify with the consumer key

dobey (dobey) wrote :

It sounds like changing reviewer_username to always be the consumer_key value, as we have for click reviews, would be the best route, and would not really break software-center (which is already somewhat broken in how it does this anyway).

With this suggested change, a fairly small change could be SRUed to software-center as well, which would even make the software-center experience better than it currently is, as it currently only recognizes reviews from you, after you've submitted a review and it's cached your username which it read from the response, into its config file (which is of course breaks on re-installs or when going to a different machine).

Robert Ancell (robert-ancell) wrote :

In GNOME Software we have the consumer key and matching the click behaviour (only returning consumer key in reviewer_username) is ideal. At dobeys suggestion we'll make GNOME Software match reviewer_username == consumer_key which will work for the majority of users and a later server side fix will work for everyone.

Thanks!

Robert Ancell (robert-ancell) wrote :

Also option 2 (adding a consumer_key to the JSON) is acceptable - up to you which one is easier.

@noodles: A long time ago, RnR started using SSO v2[0]. Prior to that, through the v1 SSO API, the LP username was *sometimes* available and it was used when it was. Since SSO v2 API account details doesn't really provide a possible username, for new reviewers the consumer_key is what's being used (as it's really the unique identifier for a particular username). This means that for *new* users (after the change!), the consumer_key would be the `User.username` (and hence the `reviewer_username` field value for the newer instances).

Later, at some point, the click reviews needed this inconsistency fixed and this branch was born[1]. In there we changed the `reviewer_username` field to always return the consumer_key (regardless of the `User.username`). My vote, for consistency and simplicity, is to go ahead and do exactly the same for deb reviews as suggested by @dobey.

[0] https://code.launchpad.net/~fgallina/rnr-server/ssov2/+merge/227058
[1] https://code.launchpad.net/~fgallina/rnr-server/click-reviews-return-useroid-as-username/+merge/257106

Changed in rnr-server:
status: New → Fix Committed
tags: added: xenial
Changed in ubuntu-release-notes:
status: New → Fix Released
tags: added: trello-import
Will Cooke (willcooke) on 2016-04-25
tags: removed: trello-import
Changed in rnr-server:
status: Fix Committed → Fix Released
Robert Ancell (robert-ancell) wrote :

Confirmed this is now working, thanks!

Changed in gnome-software (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers