Don't list apps installed during review in Account History for Reviewers

RESOLVED FIXED in 2012-12-13

Status

Marketplace
Consumer Pages
P3
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: krupa, Assigned: Andy McKay)

Tracking

2012-12-13
Points:
---

Details

(URL)

(Reporter)

Description

6 years ago
steps to reproduce:
1. As a Reviewer, go to https://marketplace-dev.allizom.org/en-US/reviewers/apps/review/communities?num=6
2. Install the app
3. Navigate to your Account History page.

expected behavior:
We don't list apps installed from the Reviewer Tools in the Account History page.

actual behavior:
The app gets listed in the Account History. For an active reviewer, this would result in hundreds of apps (with expired receipts) getting listed in their Account History.
good idea!
Priority: -- → P3
No longer blocks: 752013
(Reporter)

Comment 2

6 years ago
This still happens. We need to fix this before it becomes an issue.
Assignee: nobody → mattbasta
Target Milestone: --- → 2012-11-15

Comment 3

6 years ago
So I don't know if I'm the most qualified person to look at this. Andy might know better how to make this work.

From what I can tell, removing the installed flag for an app for a user will break the ability to create a receipt for the installation, meaning reviewers won't be able to test paid apps (if I'm reading this correctly).

The code in question is here:

https://github.com/mozilla/zamboni/blob/master/mkt/receipts/views.py

I'm not sure what's required to make the (paid?) apps work.
(Assignee)

Comment 4

6 years ago
Reviewers should be getting special receipts that don't require an entry into Installed to work. So we should be able to remove this without issue.

It looks like we are creating an Installed record however here:

https://github.com/mozilla/zamboni/blob/master/mkt/receipts/views.py#L170

I'm betting we can do some security checks and make sure that when the receipt flavour is delicious reviewer flavour, we don't create that installed record. This will require altering the create_receipt method:

https://github.com/mozilla/zamboni/blob/master/mkt/receipts/utils.py#L18

So that we don't pass in the (now non-existing) installed record.

Updated

6 years ago
Target Milestone: 2012-11-15 → 2012-11-22

Comment 5

6 years ago
Yeah, I'm pretty sure that's not going to work:

1. The receipt verifier requires that there's a user_install record that matches the UUID of the receipt:

https://github.com/mozilla/zamboni/blob/master/services/verify.py#L94

2. create_receipt requires a user_install record in order to sign the receipt:

https://github.com/mozilla/zamboni/blob/master/mkt/receipts/utils.py#L18

I've already got code that works around this, but the UUID that's pushed into the receipt isn't something that I can just fake, unless we make a constant that has a special "reviewer UUID".


Is there a convenient (i.e.: not hacked-up) way to work around this?

Updated

6 years ago
Target Milestone: 2012-11-22 → 2012-11-29

Updated

6 years ago
Target Milestone: 2012-11-29 → 2012-12-06
Andy - can you and robhudson have a look at this? Rob is doing all the reviewer tools work and you'll know how the receipt work best
Assignee: mattbasta → amckay
Target Milestone: 2012-12-06 → 2012-12-13
(Assignee)

Comment 7

6 years ago
How about adding a column to the installed table that shows who the receipt was created for. That will allow us to track what installer looked at what app and also allow installers to install as either a user or reviewer.

Then we filter the list of apps to ignore reviewer installs.

The only downside is that statistics should probably ignore this table, but they have to do that already.
(Assignee)

Comment 8

6 years ago
https://github.com/andymckay/zamboni/commit/0ff951
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.