Skip to content

Move ajax_upload_image's image checks into a helper (Sonar S3776) - #564

Merged
mzeier merged 1 commit into
stage-eksfrom
419/r3-devhub-views-998
Oct 10, 2026
Merged

mzeier merged 1 commit into
stage-eksfrom
419/r3-devhub-views-998

Conversation

@mzeier

@mzeier mzeier commented Oct 10, 2026

Copy link
Copy Markdown

Last round 3 item for thunderbird/addons-server#419. The stage-eks analysis at 02:28Z (a5b5fc6) still shows python:S3776 AaEh91ckMSBE_HpBvXpP at src/olympia/devhub/views.py:998: ajax_upload_image at cognitive complexity 17 after the #539 split.

Change, no behavior change

_uploaded_image_errors(upload_preview, upload_type, is_preview) takes the block that builds ImageCheck, calls is_animated() (which caches is_image()), and runs the type, size, persona, preview and icon checks, with the waffle switch read at the same point. The block moves verbatim and returns the error list, which the view extends into errors.

  • It's called where the block was: after the temporary file is written to storage, and before the unlink-on-error and the response.
  • is_preview (upload_type == 'preview', a pure comparison) is computed in the view, since the unlink check still needs it, and passed in. is_icon and is_persona move into the helper.
  • The else branch ("There was an error uploading your preview."), the upload_hash reset on errors and the returned dict are untouched.
  • Module globals (amo_utils, waffle, storage, ...) are still looked up at call time.

Decorators checked: ajax_upload_image still carries @json_view, directly above its def, and upload_image keeps @dev_required. _uploaded_image_errors is undecorated. The AST decorator comparison against stage-eks shows 0 changes.

Sonar issues closed

  • AaEh91ckMSBE_HpBvXpP python:S3776 src/olympia/devhub/views.py:998

Tests

flake8 is clean on changed lines. CI pytest covers devhub/tests/test_views_edit.py (the upload_image tests). I'll check this PR's Sonar analysis shows no S3776 left in devhub/views.py. Local Greptile review skipped: org quota.

Sonar python:S3776: ajax_upload_image was still at cognitive complexity
17 after #539. The ImageCheck setup and the type, size and dimension
checks move verbatim into _uploaded_image_errors(), called after the
temporary file is written, as before. The cleanup, the
'There was an error uploading your preview.' branch and the response
stay in the view. @json_view stays on ajax_upload_image.

Refs #419
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

Python Test Results

5 763 tests  ±0   5 709 ✅ ±0   12m 5s ⏱️ + 1m 13s
    1 suites ±0      54 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 255ad34. ± Comparison against base commit a5b5fc6.

@mzeier
mzeier merged commit 0dd650e into stage-eks Oct 10, 2026
3 checks passed
@mzeier
mzeier deleted the 419/r3-devhub-views-998 branch October 10, 2026 02:53
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.

1 participant