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

PNI creepy ratings on non-English pages is in the wrong order #7502

Closed
Pomax opened this issue Sep 27, 2021 · 5 comments · Fixed by #7555
Closed

PNI creepy ratings on non-English pages is in the wrong order #7502

Pomax opened this issue Sep 27, 2021 · 5 comments · Fixed by #7555
Assignees
Labels
backend bug buyer's guide 🛍 Issues related to the buyer's guide engineering

Comments

@Pomax
Copy link
Contributor

Pomax commented Sep 27, 2021

How to Reproduce:

This was flagged on Twitter https://twitter.com/dtdan03/status/1441837328323031044 and was confirmed by staff

@Pomax
Copy link
Contributor Author

Pomax commented Sep 29, 2021

Looking at the DOM, the code shows:

image

However, looking at Viber (the first item) in the database, it has 224 votes, with a spread of 22%, 21%, 36%, 10%, and 11% which means it cannot possibly have an average creepiness of 0,18834080717488788

@Pomax
Copy link
Contributor Author

Pomax commented Sep 29, 2021

Looking at the stats for respective locales, it appears that voting is storing values "for localized products" independently of the original English product, So that's not great. Solving this one will require:

  • updating the POST handling to make sure that given a product ID, we find the original product to update the votes for
  • update the product model to find the original product on which to check the creepiness values
  • write migrations that merge votes from all locales back to the original product.

@Pomax
Copy link
Contributor Author

Pomax commented Sep 29, 2021

Current code in products.py:

    @property
    def total_vote_count(self):
        return sum(self.get_or_create_votes())

    @property
    def creepiness(self):
        try:
            average = self.creepiness_value / self.total_vote_count
        except ZeroDivisionError:
            average = 50
        return average

This needs those self replaced with product, based on finding the "real" product given a localized product.

Similarly, the POST code currently reads:

        if request.body and request.method == "POST":
            data = json.loads(request.body)
            if data.get("value"):
                ...
                try:
                    product = ProductPage.objects.get(pk=self.id)
                    ...

where this product, too, should be resolved to the original English product, not the localized product.

@Pomax
Copy link
Contributor Author

Pomax commented Oct 4, 2021

Looking at the stage and prod db, I think we can get away with code-fixes only, there's a whole bunch of "zero vote" records in the database, but as far as I can they're not actually linked to any products (anymore?). So the PR for this will be code + tests without a cleanup management command. However, I will be filing a cleanup issue for "when we have time for cleanup" to see if we can safely remove those records from the database.

@Pomax
Copy link
Contributor Author

Pomax commented Oct 4, 2021

Also, given that we're going to have to add a secondary locale as part of the test framework before can even write tests this works correctly in an automated fashion, I'm going to file that as separate issue, too, and we can land this first with manual testing, then figure out how to do secondary locales (in both unit tests and load_fake_data) separaterly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend bug buyer's guide 🛍 Issues related to the buyer's guide engineering
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant