Skip to content

Reduce the amount of Reports a user can make#1194

Open
Roffenlund wants to merge 1 commit intomasterfrom
report-package-abuse-protection
Open

Reduce the amount of Reports a user can make#1194
Roffenlund wants to merge 1 commit intomasterfrom
report-package-abuse-protection

Conversation

@Roffenlund
Copy link
Copy Markdown
Contributor

Redeuce the amount of Reports a user can create per package listing to one with exception for superusers.

Refs. TS-2742

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

This PR adds a pre-check in the report_package_listing service to detect existing active, non-automated reports by the same non-superuser for a given package_listing. If such a report exists, the service raises PermissionValidationError with "You have already reported this package listing". Tests are added to verify successful first reports and to ensure duplicate report attempts are rejected at both the service and API levels.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Reduce the amount of Reports a user can make" directly summarizes the main objective of the changeset. The PR implements a pre-check to prevent users from creating multiple reports for the same package listing, with an exception for superusers. The title is concise, clear, and accurately reflects this primary change without being vague or overly broad.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch report-package-abuse-protection

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 783ebf9 and 90d4b14.

📒 Files selected for processing (3)
  • django/thunderstore/api/cyberstorm/services/package_listing.py (1 hunks)
  • django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py (2 hunks)
  • django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py
  • django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py
🧰 Additional context used
🧬 Code graph analysis (1)
django/thunderstore/api/cyberstorm/services/package_listing.py (2)
django/thunderstore/ts_reports/models/package_report.py (1)
  • PackageReport (14-88)
django/thunderstore/core/exceptions.py (1)
  • PermissionValidationError (4-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Build docker image
  • GitHub Check: Build docker image
🔇 Additional comments (1)
django/thunderstore/api/cyberstorm/services/package_listing.py (1)

71-80: Race condition on concurrent report submissions.

Two requests arriving simultaneously can both pass the existence check before either completes, bypassing the deduplication. There's no database constraint to prevent this—only application-level logic. Whether this matters depends on your acceptable duplicate rate for a reporting system; if duplicates must be prevented, add a database-level unique constraint on (submitted_by, package_listing) where is_automated=False and is_active=True.

Consider the follow-up reporting workflow.

The current implementation blocks any second report from the same user on the same listing. Users often need to submit follow-up information after an initial report. You may want to allow updates to existing reports or permit reports on the same listing with different reasons instead.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.81%. Comparing base (5f7c539) to head (90d4b14).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1194   +/-   ##
=======================================
  Coverage   92.81%   92.81%           
=======================================
  Files         337      337           
  Lines       10355    10359    +4     
  Branches      937      938    +1     
=======================================
+ Hits         9611     9615    +4     
  Misses        617      617           
  Partials      127      127           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f7c539 and 783ebf9.

📒 Files selected for processing (3)
  • django/thunderstore/api/cyberstorm/services/package_listing.py (1 hunks)
  • django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py (2 hunks)
  • django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py (4)
django/thunderstore/api/cyberstorm/services/package_listing.py (1)
  • report_package_listing (61-89)
django/thunderstore/core/exceptions.py (1)
  • PermissionValidationError (4-7)
django/thunderstore/ts_reports/models/package_report.py (1)
  • PackageReport (14-88)
django/conftest.py (2)
  • user (116-121)
  • active_package_listing (245-249)
django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py (2)
django/conftest.py (3)
  • user (116-121)
  • TestUserTypes (566-601)
  • get_user_by_type (580-601)
django/thunderstore/api/cyberstorm/views/package_listing_actions.py (5)
  • post (56-71)
  • post (84-101)
  • post (114-130)
  • post (142-161)
  • post (173-180)
django/thunderstore/api/cyberstorm/services/package_listing.py (2)
django/thunderstore/ts_reports/models/package_report.py (1)
  • PackageReport (14-88)
django/thunderstore/core/exceptions.py (1)
  • PermissionValidationError (4-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Build docker image
  • GitHub Check: Build docker image
🔇 Additional comments (2)
django/thunderstore/api/cyberstorm/services/package_listing.py (1)

71-80: LGTM!

The duplicate report check is correctly implemented. The logic filters for existing non-automated, active reports by the same user for the listing, and exempts superusers as intended. The error message is clear and user-friendly.

django/thunderstore/api/cyberstorm/tests/services/test_package_listing_services.py (1)

179-230: LGTM!

Both tests are well-structured and comprehensive. test_report_package_listing_success verifies the happy path, while test_report_package_listing_report_limit confirms that duplicate reports are properly rejected and only a single report exists afterward.

Comment thread django/thunderstore/api/cyberstorm/tests/test_package_listing_actions.py Outdated
@x753
Copy link
Copy Markdown
Contributor

x753 commented Oct 19, 2025

This implementation is a bad idea. People frequently make reports with no reason, then make a second report later with an updated reason.

Maybe make it so you can only have one report for a given reason and additional reports are updated and appended to the description? Or just leave things as they are, as any sort of spam is better addressed globally rather than on a per-object basis.

Redeuce the amount of Reports a user can create per package listing to
one with exception for superusers.

Refs. TS-2742
@Roffenlund Roffenlund force-pushed the report-package-abuse-protection branch from 783ebf9 to 90d4b14 Compare October 20, 2025 06:59
@Roffenlund
Copy link
Copy Markdown
Contributor Author

This implementation is a bad idea. People frequently make reports with no reason, then make a second report later with an updated reason.

Maybe make it so you can only have one report for a given reason and additional reports are updated and appended to the description? Or just leave things as they are, as any sort of spam is better addressed globally rather than on a per-object basis.

I don’t have specific requirements for this yet, other than the need to restrict reports. The goal was to use this as a starting point for discussion and to further refine the task. You suggestion sounds like a good idea.

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.

2 participants