Community Health: Obsolete resources and corresponding entities

[functionzz]

Summary

This PR addresses the data loss incurred when someone removes strings from projects like Firefox. Instead of obsoleting the strings, we currently delete the data which causes contributior attribution to be lost.

Fixes #2133.

[mathjazz]

Nice work!

Please also make sure that everywhere in the code where we references all resources (and count on the fact that obsolete resources are currently not present), we now reference active resources only.

[mathjazz]

Thanks for the update!

This will need extensive testing before we ship it.

[functionzz]

I guess the next natural step is writing tests for the various cases listed here if everything looks fine.

[mathjazz]

I guess the next natural step is writing tests for the various cases listed here if everything looks fine.

That would be great!

[functionzz]

@eemeli, I would like some guidance on how to test these .current() properties. Since all previous tests pass. I'm finding it difficult to come up with tests without just testing how .current() works over and over again.

[eemeli]

Looks good; see inline for some relatively minor things.

I would like some guidance on how to test these .current() properties. Since all previous tests pass. I'm finding it difficult to come up with tests without just testing how .current() works over and over again.

Eh, What's here is already pretty decent. I would like to see something exercising TranslatedResource.objects.current() after ResourceQuerySet.mark_as_obsolete() is called, but that's about it.

[eemeli]

I'll leave this open in case @mathjazz wants to take another look at it once he's back, but I think this is good to merge.

[mathjazz]

The patch is deployed to the dev instance:
https://pontoon.allizom.org/

We should also test it with sync.

[flodolo]

Tried throwing an LLM at this, might be worth checking?

pontoon/tags/utils.py:104 (for the Tags class)

  @property
  def translated_resources(self):
      trs = TranslatedResource.objects  # ← should be .current()
      if self.project is not None:
          trs = trs.filter(resource__project=self.project)
      if self.locale is not None:
          trs = trs.filter(locale=self.locale)
      return trs

---

pontoon/sync/core/entities.py:136

    changed_resources: dict[int, Resource] | None = (
        {res.pk: res for res in project.resources.filter(path__in=updates.keys())}
        if updates
        else None
    )

What happens if a string was previously marked as obsolete, but comes back?

---

pontoon/sync/core/entities.py:293 and pontoon/base/models/project.py:310

  # entities.py:293 (add_resources)
  ordered_resources = project.resources.order_by("path")

  # project.py:310 (reset_resource_order)
  for idx, r in enumerate(self.resources.order_by("path")):

Should we exclude obsolete resources here, even if practically it doesn't matter?

[flodolo]

Possibly a few more.

Are these counting obsolete resources?
https://github.com/mozilla/pontoon/blob/main/pontoon/base/models/project_locale.py#L33-L45
https://github.com/mozilla/pontoon/blob/main/pontoon/base/models/locale.py#L65-L72

[functionzz]

Applied the first two changes.

What happens if a string was previously marked as obsolete, but comes back?

If it comes back, it should come back as a new string, not an un-obsoleted version of the old string.

Should we exclude obsolete resources here, even if practically it doesn't matter?

Let's not add .current() if we don't need it, since there are places that require just Resource.objects to function.

Are these counting obsolete resources? (Referring to stats_data methods)

Yes, these are, although they should not be, as these refer to Locale and Project_Locale statistics, which should reflect current localizations.