Skip to content

sync enforcement actions from Cinder for policies#23161

Merged
eviljeff merged 2 commits intomozilla:masterfrom
eviljeff:15421-sync-enforcement-actions
Mar 14, 2025
Merged

sync enforcement actions from Cinder for policies#23161
eviljeff merged 2 commits intomozilla:masterfrom
eviljeff:15421-sync-enforcement-actions

Conversation

@eviljeff
Copy link
Copy Markdown
Member

@eviljeff eviljeff commented Mar 12, 2025

Fixes: mozilla/addons#15421

Description

Adds support for the cinder policy sync to import and store enforcement actions too; and then replaces the single (admin manually defined) default_cinder_action with a list of enforcement_actions.

Context

At the current time this doesn't really "fix" anything, but it's needed for https://mozilla-hub.atlassian.net/browse/AMOENG-672 and would have negated the need for https://mozilla-hub.atlassian.net/browse/ADDONSOPS-810. (That there was reviewer tools functionality that implicitly depended on manual admin settings, after an automatic sync, always felt like a footgun to me, and I wanted to resolve it).

Testing

  • Ideally, before checking this patch out, sync Cinder policies in django admin; then choose one (or more) policies to have a default_cinder_action set on them.
  • Then check out the patch, run the migrations, and check that those policies now have enforcement_actions prefilled with the default cinder action you chose.
  • Then run sync Cinder policies in django admin again, and see all* the policies now have enforcement_actions defined. *(not actually all - the parent policies don't have them, for example)
  • In reviewer tools, on the review page for an add-on, resolve an abuse report job as Invalid. This should work without form errors.
    • to set up you will need to report the add-on first as violating policies in the add-on and have all the DSA waffle switches enabled
  • Repeat, but resolve an abuse report job as Approve
  • In reviewer tools 2nd level approval queue, on a review page, make a 2nd level approval decision to Approve an add-on. Check in Cinder that this override to Approve was recorded as a decision.
    • to set up you will need to make an add-on promoted and make a reject or disable decision on a version of that add-on in the reviewer tools.

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.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@eviljeff eviljeff marked this pull request as ready for review March 12, 2025 15:23
@eviljeff eviljeff requested review from a team and diox and removed request for a team March 12, 2025 15:23
@diox
Copy link
Copy Markdown
Member

diox commented Mar 13, 2025

When resolving a report in reviewer tools, I got:

TypeError at /reviewers/review/XXXX
CinderPolicy.get_decision_actions_from_policies() takes 2 positional arguments but 3 were given
/data/olympia/src/olympia/reviewers/utils.py, line 1099, in get_decision_actions_from_policies

Which is worrying considering tests are shown as passing ?

@diox
Copy link
Copy Markdown
Member

diox commented Mar 13, 2025

Ah, the CI bug strikes again. There are actually some failures:

=========================== short test summary info ============================
FAILED src/olympia/reviewers/tests/test_forms.py::TestReviewForm::test_cinder_jobs_filtered_for_resolve_reports_job_and_resolve_appeal_job
FAILED src/olympia/reviewers/tests/test_forms.py::TestReviewForm::test_only_one_cinder_action_selected
FAILED src/olympia/reviewers/tests/test_forms.py::TestReviewForm::test_policies_required_with_cinder_jobs
FAILED src/olympia/reviewers/tests/test_utils.py::TestReviewHelper::test_record_decision_called_everywhere_checkbox_shown_listed
FAILED src/olympia/reviewers/tests/test_utils.py::TestReviewHelper::test_record_decision_called_everywhere_checkbox_shown_unlisted
FAILED src/olympia/reviewers/tests/test_utils.py::TestReviewHelper::test_record_decision_sets_policies_with_allow_policies
FAILED src/olympia/reviewers/tests/test_views.py::TestReview::test_resolve_reports_job
============ 7 failed, 533 passed, 95 warnings in 244.32s (0:04:04) ============

@eviljeff
Copy link
Copy Markdown
Member Author

Ah, the CI bug strikes again. There are actually some failures:

=========================== short test summary info ============================
FAILED src/olympia/reviewers/tests/test_forms.py::TestReviewForm::test_cinder_jobs_filtered_for_resolve_reports_job_and_resolve_appeal_job
FAILED src/olympia/reviewers/tests/test_forms.py::TestReviewForm::test_only_one_cinder_action_selected
FAILED src/olympia/reviewers/tests/test_forms.py::TestReviewForm::test_policies_required_with_cinder_jobs
FAILED src/olympia/reviewers/tests/test_utils.py::TestReviewHelper::test_record_decision_called_everywhere_checkbox_shown_listed
FAILED src/olympia/reviewers/tests/test_utils.py::TestReviewHelper::test_record_decision_called_everywhere_checkbox_shown_unlisted
FAILED src/olympia/reviewers/tests/test_utils.py::TestReviewHelper::test_record_decision_sets_policies_with_allow_policies
FAILED src/olympia/reviewers/tests/test_views.py::TestReview::test_resolve_reports_job
============ 7 failed, 533 passed, 95 warnings in 244.32s (0:04:04) ============

yeah, my usual workflow of "test obvious places and then trust CI to highlight anything I missed" has been really impacted by that bug - I'll rebase.

@eviljeff eviljeff force-pushed the 15421-sync-enforcement-actions branch from 5be1c17 to 6e2846b Compare March 13, 2025 18:34
@eviljeff eviljeff merged commit df1d35c into mozilla:master Mar 14, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: sync enforcement actions from cinder policies api endpoint

2 participants