Repository navigation
Round 3: Sonar S3776 complexity splits in reviewers/ - #558
Merged
Merged
Conversation
- reviewers/utils.py: ReviewHelper.get_actions() (complexity 21) split
into _compute_action_conditions() (all the boolean condition
definitions) and _build_actions() (the 7 action dict definitions,
keyed off the conditions dict); get_actions() now just computes
conditions, builds actions, applies the content-review-only label
tweak, and filters by availability.
- reviewers/models.py: ReviewerScore.get_event() (complexity 33) split
into _get_event_name_for_post_review() and
_get_event_name_for_regular_review() classmethods, one per branch;
get_event() now just dispatches to content_review/post_review/regular
and resolves the final constant.
- reviewers/templatetags/jinja_helpers.py: get_position() (complexity
18) split into _persona_position() and _regular_position() module
functions, bodies moved verbatim.
- reviewers/views.py:
- _queue() (complexity 19, introduced by my own prior-round edit to
this function) split into _setup_queue_search_form(),
_apply_queue_raw_sql_restrictions(), _normalize_queue_per_page().
- dashboard() (complexity 29) split into one function per section
(_dashboard_legacy_addons_section, _dashboard_auto_approved_section,
_dashboard_content_review_section,
_dashboard_ratings_moderation_section,
_dashboard_unlisted_addons_section, _dashboard_announcement_section,
_dashboard_admin_tools_section), each returning (header, section);
dashboard() now just calls the relevant one per permission check and
assigns into sections[header].
- perform_review_permission_checks() (complexity 17) split out
_check_regular_review_permission() for the non-content/non-theme
branch.
- review() (complexity 43, the single largest function in this round)
split into _check_review_permissions_unless_read_only(),
_gather_channel_review_info(), _kick_off_validation_tasks(),
_find_show_diff_version(), _build_auto_approval_info(). review()
itself keeps the same straight-line sequence of steps, now mostly
one function call per former inline block.
No behavior change anywhere in this PR: every extraction is a verbatim
body move preserving the same truth table, evaluation order, and
side-effect order (DB queries, cache writes, early returns/redirects,
and exception handling all happen in the same relative order as
before).
- _dashboard_legacy_addons_section() never used its request parameter; removed it from the signature and the call site (S1172). - Moved the "Permission checks for this view are done inside..." comment back to directly above review() (it describes that view's behavior, not _check_review_permissions_unless_read_only(), which ended up sitting where review() used to be after the earlier S3776 split).
|
mzeier
added a commit
that referenced
this pull request
Oct 10, 2026
My earlier split in #558 (merged) reduced review() from complexity 43 to 16, still 1 over the allowed 15, per re-pull against stage-eks after merge (python:S3776, reviewers/views.py:920). Extracted two more self-contained chunks, both verbatim: - _filter_review_actions(): the actions_full/actions_comments list comprehensions. - _build_whiteboard_form(): the Whiteboard get-or-construct try/except plus the PublicWhiteboardForm/WhiteboardForm class selection. review() keeps its @login_required/@addon_view_factory decorators directly above it, and the "Permission checks for this view..." comment stays directly above those decorators. No behavior change: both extractions are straight body moves with no change to evaluation order or side effects.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #450 (round 3).
reviewers/utils.py:ReviewHelper.get_actions()(S3776,reviewers/utils.py:301, complexity 21) split into_compute_action_conditions()(all the boolean condition definitions) and_build_actions()(the 7 action dict definitions, keyed off the conditions dict);get_actions()now just computes conditions, builds actions, applies the content-review-only label tweak, and filters by availability.reviewers/models.py:ReviewerScore.get_event()(S3776,reviewers/models.py:420, complexity 33) split into_get_event_name_for_post_review()and_get_event_name_for_regular_review()classmethods, one per branch;get_event()now just dispatches to content_review/post_review/regular and resolves the final constant.reviewers/templatetags/jinja_helpers.py:get_position()(S3776,reviewers/templatetags/jinja_helpers.py:178, complexity 18) split into_persona_position()and_regular_position()module functions, bodies moved verbatim.reviewers/views.py:_queue()(S3776,reviewers/views.py:460, complexity 19 - this one was introduced by my own prior-round edit to this function, per the re-pull before starting) split into_setup_queue_search_form(),_apply_queue_raw_sql_restrictions(),_normalize_queue_per_page().dashboard()(S3776,reviewers/views.py:149, complexity 29) split into one function per section (_dashboard_legacy_addons_section,_dashboard_auto_approved_section,_dashboard_content_review_section,_dashboard_ratings_moderation_section,_dashboard_unlisted_addons_section,_dashboard_announcement_section,_dashboard_admin_tools_section), each returning(header, section);dashboard()now just calls the relevant one per permission check and assigns intosections[header].perform_review_permission_checks()(S3776,reviewers/views.py:695, complexity 17) split out_check_regular_review_permission()for the non-content/non-theme branch.review()(S3776,reviewers/views.py:759, complexity 43, the single largest function in this round) split into_check_review_permissions_unless_read_only(),_gather_channel_review_info(),_kick_off_validation_tasks(),_find_show_diff_version(),_build_auto_approval_info().review()itself keeps the same straight-line sequence of steps, now mostly one function call per former inline block.No behavior change anywhere in this PR: every extraction is a verbatim body move preserving the same truth table, evaluation order, and side-effect order (DB queries, cache writes, early returns/redirects, and exception handling all happen in the same relative order as before).
Verified no duplicate function names and every new helper is called exactly once (
grep -csanity check across the file).Local Greptile review: org free-tier quota exhausted, skipped per coordinator guidance; the Greptile GitHub check on this PR still runs independently.