Log name|summary changes in more detail

[eviljeff]

Fixes: mozilla/addons#15742

Description

Name and summary addon metadata changes create EDIT_ADDON_PROPERTY activity logs, with the added and removed localized strings in a json blob in the arguments (not displayed).

A simple name change from en-US: good add-on to en-US: bad add-on would result in the following activity log record:

action = amo.LOG.EDIT_ADDON_PROPERTY.id
arguments = [<Addon: id>, 'name', '{"added": ["bad add-on"], "removed": ["good add-on"]}']

or in the database:
action = 208
arguments = '[{"addons.addon": 1}, {"str": "name"}, {"str": "{\\"added\\": [\\"bad add-on\\"], \\"removed\\": [\\"good add-on\\"]}"}]

Added and removed are de-duplicated lists, and if a localized string hasn't changed it won't be included.

Context

I originally intended to replace EDIT_PROPERTIES everywhere, and create seperate EDIT_ADDON_PROPERTY logs for each property change but ran into some difficulties with the devhub form (localized fields weren't being returned in changed_fields property) so, given the priority on the issue, limited the change to name and summary - and added the same limitation to AddonSerializer for consistency.

Testing

• In devhub, edit the name (or summary) of an add-on with listed versions in en-US locale and save the form

• check:

  • the name (or summary) has changed
  • there is an activity log in the developer's activity log view for "{addon} name property edited."
  • in a django shell (or dbshell) check the details json blob contains the old and new names (or summaries) for the EDIT_ADDON_PROPERTY activity

• repeat via the API, by submitting a PATCH to the addon endpoint, changing the name (or summary)

• repeat the checks:

  • the name (or summary) has changed
  • there is an activity log in the developer's activity log view for "{addon} name property edited."
  • in a django shell (or dbshell) check the details json blob contains the old and new names (or summaries)

• Optional further testing:

  • add and/or delete name/summary in multiple, other locales
  • duplicate some strings (name is commonly duplicated because it's often a brand name)
  • change other properties at the same time and see the older EDIT_PROPERTIES is used for other properties, and name/summary in add-ons without listed versions.

Checklist

• Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
• Successfully verified the change locally.
• The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.

[KevinMind]

FYI you can link directly to the swagger and API calls can be made easily via the browser.

http://olympia.test/api/v5/swagger/#/addons/addons_addon_partial_update

[KevinMind]

This also works in dev (soon prod)

[KevinMind]

I was able to verify via devhub relatively straight forward. Via API I see the description was updated, and I see the activity log in devhub but when checking the shell for some reason I still only see one activity log record.

I'm doing something like this

from olympia.constants.activity import LOG

addon = Addon.objects.get(slug="camellam-pheroach-ottle")
logs = ActivityLog.objects.for_addons(addon.id).filter(action=LOG.EDIT_ADDON_PROPERTY.id)

After updating via devhub, I got 1 record.. then when updating via API.. still one record.

[eviljeff]

It's not easy to tell where one screenshot ends and another starts - some text labels would help break the images.

But I think from what I see it's because you're trying to update a field that requires a object with a string.

[KevinMind]

I've verified the main flow, we can verify further on stage if we want to.

[eviljeff]

I've verified the main flow, we can verify further on stage if we want to.

What is the "main flow"? Do you mean you've verified it works, with both the API and devhub?

[KevinMind]

Responded offline. I was able to verify the patch according to the steps. I only ran into issues inspecting the activity logs via the djshell. I was able to see the first log for an addon, but after making further changes to name/description I still only saw the one log.

[eviljeff]

Responded offline. I was able to verify the patch according to the steps. I only ran into issues inspecting the activity logs via the djshell. I was able to see the first log for an addon, but after making further changes to name/description I still only saw the one log.

it's name and summary - description isn't one of the fields tracked for content review purposes so isn't included in this (though could easily be, if Product/Operations want to expand this to other metadata)