Skip to content

drop CinderJob.decision property #24864

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.

Sign up for GitHub

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

Open
eviljeff wants to merge 1 commit into
base: master
Choose a base branch
from
Open

drop CinderJob.decision property #24864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions src/olympia/abuse/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ class Meta:
models.CheckConstraint(name='no_empty_job_id', check=~Q(job_id=''))
]

@property
def decision(self):
"""This is the first decision made on the job"""
return self.decisions.first()

@property
def final_decision(self):
return self.decisions.last()
Expand All @@ -119,9 +114,9 @@ def target(self):
if self.target_addon_id:
# if this was a job from an abuse report for an addon we've set target_addon
return self.target_addon
elif decision := self.decision:
elif final_decision := self.final_decision:
# if there is already a decision target will be set there
return decision.target
return final_decision.target
elif self.is_appeal:
# if this is an appeal job the decision will have the target
return self.appealed_decisions.first().target
Expand Down Expand Up @@ -393,7 +388,7 @@ def clear_needs_human_review_flags(self):
# isn't resolved yet, but there is no link between NHR and jobs.
# So for each possible reason, we look if there are unresolved jobs
# and only clear NHR for that reason if there aren't any jobs left.
addon = self.decision.addon
addon = self.final_decision.addon
base_unresolved_jobs_qs = (
self.__class__.objects.for_addon(addon)
.unresolved()
Expand Down
28 changes: 14 additions & 14 deletions src/olympia/abuse/tests/test_actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,11 @@ def test_ban_user_after_reporter_appeal(self):
user=self.user, action=DECISION_ACTIONS.AMO_APPROVE
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
subject = self._test_ban_user()
assert len(mail.outbox) == 2
Expand Down Expand Up @@ -715,11 +715,11 @@ def test_execute_action_after_reporter_appeal(self):
addon=self.addon, action=DECISION_ACTIONS.AMO_APPROVE
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
subject = self._process_action_and_notify()
assert len(mail.outbox) == 2
Expand Down Expand Up @@ -1447,11 +1447,11 @@ def test_execute_action_after_reporter_appeal(self):
addon=self.addon, action=DECISION_ACTIONS.AMO_APPROVE
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
subject = self._test_reject_version(content_review=False)
assert len(mail.outbox) == 2
Expand Down Expand Up @@ -1605,11 +1605,11 @@ def test_execute_action_delayed_after_reporter_appeal(self):
action=DECISION_ACTIONS.AMO_APPROVE, addon=self.addon
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
subject = self._test_reject_version_delayed(content_review=False)
assert len(mail.outbox) == 2
Expand Down Expand Up @@ -2404,11 +2404,11 @@ def test_reporter_appeal_approve(self):
action=DECISION_ACTIONS.AMO_APPROVE,
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
self.cinder_job.reload()
subject = self._test_reporter_content_approved_action_taken()
Expand Down Expand Up @@ -2813,11 +2813,11 @@ def test_delete_collection_after_reporter_appeal(self):
collection=self.collection, action=DECISION_ACTIONS.AMO_APPROVE
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
subject = self._test_delete_collection()
assert len(mail.outbox) == 2
Expand Down Expand Up @@ -2989,11 +2989,11 @@ def test_delete_rating_after_reporter_appeal(self):
rating=self.rating, action=DECISION_ACTIONS.AMO_APPROVE
),
)
self.cinder_job.appealed_decisions.add(original_job.decision)
self.cinder_job.appealed_decisions.add(original_job.final_decision)
self.abuse_report_no_auth.update(cinder_job=original_job)
self.abuse_report_auth.update(cinder_job=original_job)
CinderAppeal.objects.create(
decision=original_job.decision, reporter_report=self.abuse_report_auth
decision=original_job.final_decision, reporter_report=self.abuse_report_auth
)
subject = self._test_delete_rating()
assert len(mail.outbox) == 2
Expand Down
14 changes: 8 additions & 6 deletions src/olympia/abuse/tests/test_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ def test_auto_resolve_reviewed_handled(self):

assert CinderJob.objects.unresolved().count() == 1
assert CinderJob.objects.unresolved().get() == job_not_reviewed
assert job1.reload().decision
assert job1.decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
assert job2.reload().decision
assert job2.decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
assert job1.reload().final_decision
assert job1.final_decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
assert job2.reload().final_decision
assert job2.final_decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
# NHRs should be cleared.
assert not NeedsHumanReview.objects.filter(
version__addon=addon1, is_active=True
Expand Down Expand Up @@ -196,8 +196,10 @@ def test_auto_resolve_forwarded_job(self):
job_appeal,
job_second_level,
]
assert job_forwarded.reload().decision
assert job_forwarded.decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
assert job_forwarded.reload().final_decision
assert (
job_forwarded.final_decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
)
# NHR should be cleared.
assert not NeedsHumanReview.objects.filter(
version__addon=addon1, is_active=True
Expand Down
Loading
Loading