diff --git a/badges/sources.py b/badges/sources.py index 8ca568775..277861047 100644 --- a/badges/sources.py +++ b/badges/sources.py @@ -16,9 +16,28 @@ against the current models would not survive it. All three can still be granted by hand in the admin. + +Sub-libraries (``math/quaternion``, ``functional/hash``, and the rest of +``SUB_LIBRARIES``) are excluded from every library-shaped source. They are +subdivisions of a parent library's documentation rather than libraries of their +own, so only the parent counts, and authorship of a sub-library alone earns +nothing here. Recognising that work would take a badge of its own. """ from badges.enums import AchievementSlug +from libraries.constants import SUB_LIBRARIES + + +def _iter_library_authoring(): + """Yield (user, library) for every authorship of a parent library.""" + from libraries.models import Library + + libraries = Library.objects.exclude(key__in=SUB_LIBRARIES).prefetch_related( + "authors" + ) + for library in libraries.iterator(chunk_size=500): + for user in library.authors.all(): + yield user, library def _iter_code_commits(): @@ -35,6 +54,7 @@ def _iter_code_commits(): BACKFILL_ITERATORS = { + AchievementSlug.LIBRARY_AUTHORING: _iter_library_authoring, AchievementSlug.CODE_COMMITS: _iter_code_commits, } diff --git a/badges/tests/test_commands.py b/badges/tests/test_commands.py index c56969539..30672265a 100644 --- a/badges/tests/test_commands.py +++ b/badges/tests/test_commands.py @@ -73,6 +73,18 @@ def test_backfill_skips_recalculation_without_new_rows(plain_user): assert not UserBadge.objects.exists() +def test_backfill_library_authoring(plain_user): + """Backfill grants the authoring achievement from Library authors.""" + library = baker.make("libraries.Library") + library.authors.add(plain_user) # M2M only - no live signal grants it + + call_command("backfill_achievements", "--source", "library-authoring") + + assert UserBadge.objects.filter( + user=plain_user, badge__achievement__slug="library-authoring" + ).exists() + + def test_backfill_fails_loudly_on_an_explicit_unseeded_source(plain_user): """A named source with no Achievement row is a deploy bug, not a skip.""" Achievement.objects.filter(slug=AchievementSlug.CODE_COMMITS).delete() @@ -81,6 +93,20 @@ def test_backfill_fails_loudly_on_an_explicit_unseeded_source(plain_user): call_command("backfill_achievements", "--source", "code-commits") +def test_backfill_skips_an_unseeded_source_on_a_full_run(plain_user, capsys): + """A scheduled sweep must not lose every other source to one missing row.""" + library = baker.make("libraries.Library") + library.authors.add(plain_user) + Achievement.objects.filter(slug=AchievementSlug.CODE_COMMITS).delete() + + call_command("backfill_achievements") + + assert "code-commits" in capsys.readouterr().err + assert UserBadge.objects.filter( + user=plain_user, badge__achievement__slug="library-authoring" + ).exists() + + def test_backfill_fails_when_no_source_is_seeded(plain_user): """Nothing to back fill at all is still worth a non-zero exit.""" Achievement.objects.all().delete() @@ -295,6 +321,25 @@ def test_reconcile_rejects_an_unknown_member(plain_user): call_command("reconcile_achievements", "--user", "nobody@example.com") +def test_reconcile_scopes_to_the_named_source( + plain_user, commit_by_someone_else, stale_commit_grant +): + """``--source`` must not walk, or prune, a source it was not given.""" + library = baker.make("libraries.Library") + library.authors.add(plain_user) + call_command("backfill_achievements", "--source", "library-authoring") + library.authors.remove(plain_user) + + call_command("reconcile_achievements", "--source", "code-commits") + + assert not UserAchievement.objects.filter( + user=plain_user, achievement__slug="code-commits" + ).exists() + assert UserAchievement.objects.filter( + user=plain_user, achievement__slug="library-authoring" + ).exists() + + def test_reconcile_refuses_a_source_that_yields_nothing( plain_user, stale_commit_grant, capsys ): diff --git a/badges/tests/test_sources.py b/badges/tests/test_sources.py index 31f34ef86..b0e5b8ca8 100644 --- a/badges/tests/test_sources.py +++ b/badges/tests/test_sources.py @@ -46,6 +46,26 @@ def test_an_automatic_grant_is_idempotent(plain_user): ) +def test_iter_library_authoring(plain_user): + """The authoring iterator yields each (author, library) pair.""" + library = baker.make("libraries.Library") + library.authors.add(plain_user) + pairs = list(sources._iter_library_authoring()) + assert (plain_user, library) in pairs + + +def test_iter_library_authoring_skips_sub_libraries(plain_user): + """A sub-library is its parent's, so authoring one alone counts for nothing. + + ``math/quaternion`` and the rest of ``SUB_LIBRARIES`` are subdivisions of a + parent library's documentation, and the badge counts parent libraries. + """ + sub = baker.make("libraries.Library", key="math/quaternion") + sub.authors.add(plain_user) + + assert list(sources._iter_library_authoring()) == [] + + def test_iter_code_commits_skips_unlinked(plain_user): """Only commits whose author has a linked user are yielded.""" linked = baker.make("libraries.CommitAuthor", user=plain_user) diff --git a/badges/tests/test_tasks.py b/badges/tests/test_tasks.py index 12e60a96d..085a33ebe 100644 --- a/badges/tests/test_tasks.py +++ b/badges/tests/test_tasks.py @@ -18,6 +18,19 @@ def test_backfill_task_sweeps_every_source_by_default(catalogue, capsys): assert f"{slug}:" in output +def test_backfill_task_scopes_to_one_source(catalogue, capsys): + """A slug reaches the command as ``--source`` rather than being ignored. + + The task passes it by the argument's ``dest``, a different word from the + option, so the only proof it landed is one source running and the others not. + """ + backfill_achievements_task(slug="library-authoring") + + output = capsys.readouterr().out + assert "library-authoring:" in output + assert "code-commits:" not in output + + def test_reconcile_task_scopes_its_run_to_one_member( stale_commit_grant, commit_by_someone_else, plain_user, super_user ): diff --git a/core/tests/test_admin_buttons.py b/core/tests/test_admin_buttons.py index f866e7c60..50ba865ca 100644 --- a/core/tests/test_admin_buttons.py +++ b/core/tests/test_admin_buttons.py @@ -438,3 +438,23 @@ def test_status_names_the_source_of_a_scoped_run(client, super_user): body = client.get(reverse(CHANGELIST_URL)).content.decode() assert "Backfill achievements (Code Commits): Running" in body + + +def test_scoped_lock_is_per_argument(client, super_user): + """One source running does not refuse a run for another. + + They are different jobs; they only share a button. + """ + client.force_login(super_user) + cache.set( + f"{LAST_RUN_KEY}:job:code-commits", "running-task", TASK_BUTTON_COOLDOWN_SECONDS + ) + + with _state("STARTED"), patch(TASK_PATH, return_value=_result()) as delay: + refused = client.post( + reverse(BACKFILL_URL), {"slug": "code-commits"}, follow=True + ) + client.post(reverse(BACKFILL_URL), {"slug": "library-authoring"}) + + assert "not starting another one" in refused.content.decode() + delay.assert_called_once_with(slug="library-authoring", actor_id=super_user.pk)