Doesn't recognise own reviews

Bug #1564209 reported by Robert Ancell
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Ratings and Reviews server
Fix Released
Undecided
Unassigned
Release Notes for Ubuntu
Fix Released
Undecided
Unassigned
gnome-software (Ubuntu)
Fix Released
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.

Tags: xenial

Related branches

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
Martin Albisetti (beuno) wrote :

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

Revision history for this message
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

Revision history for this message
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).

Revision history for this message
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!

Revision history for this message
Robert Ancell (robert-ancell) wrote :

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

Revision history for this message
Fabián Ezequiel Gallina (fgallina) wrote :

@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
Mathew Hodson (mhodson)
tags: added: xenial
Mathew Hodson (mhodson)
Changed in ubuntu-release-notes:
status: New → Fix Released
tags: added: trello-import
Will Cooke (willcooke)
tags: removed: trello-import
Changed in rnr-server:
status: Fix Committed → Fix Released
Revision history for this message
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  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.