Validate attachment rights on listing media AJAX

[sorinmarta]

Fixes https://github.com/Strategy11/business-directory-premium/issues/371
Authenticated listing image attach and delete now require per-attachment capabilities.

[coderabbitai]

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Warning

Review limit reached

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: dddab55f-d232-449a-ad51-9bf2199554d5

📥 Commits

Reviewing files that changed from the base of the PR and between 80edc74 and 9c03fda.

📒 Files selected for processing (1)

• tests/wpunit/Listing/ListingMediaAuthorizationTest.php

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4addd817-dd33-4c98-82ea-dac1781bf20c

📥 Commits

Reviewing files that changed from the base of the PR and between 79aa5fe and 80edc74.

📒 Files selected for processing (4)

• includes/class-wpbdp.php
• includes/helpers/class-listing-image.php
• templates/submit-listing-images-upload-form.tpl.php
• tests/wpunit/Listing/ListingMediaAuthorizationTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

Changes

Listing media authorization

Layer / File(s)	Summary
Image authorization contracts includes/helpers/class-listing-image.php, tests/wpunit/Listing/ListingMediaAuthorizationTest.php	Image IDs are normalized. Attachment type, ownership, and edit/delete permissions are validated.
AJAX media authorization includes/class-wpbdp.php, tests/wpunit/Listing/ListingMediaAuthorizationTest.php	Image attachment and deletion handlers now enforce capabilities and listing ownership. Selected images are filtered before the success response.
Media selector and test coverage templates/submit-listing-images-upload-form.tpl.php, tests/wpunit/Listing/ListingMediaAuthorizationTest.php	The media selector requires upload_files. Tests cover AJAX dispatch, fixtures, cleanup, authorization, deletion, and normalization.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ajax_listing_media_image
  participant WPBDP_Listing_Image
  participant Listing
  Client->>ajax_listing_media_image: submit image IDs
  ajax_listing_media_image->>WPBDP_Listing_Image: normalize and validate images
  WPBDP_Listing_Image->>Listing: check ownership and capabilities
  Listing-->>WPBDP_Listing_Image: authorization result
  ajax_listing_media_image->>Client: return filtered HTML or error

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The pull request claims to fix issue #371, but the provided issue objectives concern URL redirects, API request identification, licensing parameters, and account creation validation. They do not match…	Link the pull request to the issue that covers listing media attachment and deletion authorization, or update the linked issue and description to match the actual changes.
Linked Issues check	⚠️ Warning	Issue #371 requires coding changes for HTTPS theme-management URLs, API request user-agent identification, licensing query parameters and version handling, and account-creation validation conditions. …	Implement the coding requirements in issue #371, including the HTTPS URL, API user-agent, licensing, and account-validation changes, with automated tests. Alternatively, link this PR to the issue that defines the listing-media authorization…
Out of Scope Changes check	⚠️ Warning	The PR changes AJAX listing-image attachment and deletion authorization, adds image ownership and capability checks, gates the media selector, and adds authorization tests. These changes support listi…	Move the listing-media authorization implementation and its tests to a PR linked to the relevant issue, or update the linked issue scope before merging. Keep this PR focused on the requirements in issue #371.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The implementation, helper methods, template changes, and tests all support the stated objective of enforcing per-attachment rights for listing media AJAX operations.
Docstring Coverage	✅ Passed	Docstring coverage is 87.88% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 4 files.
Description check	✅ Passed	The description directly describes the per-attachment capability checks added to listing image attachment and deletion, which matches the changeset.
Title check	✅ Passed	The title clearly identifies the main change: validating attachment rights for listing media AJAX operations.

Full details: Linked Issues check

Explanation

The pull request claims to fix issue #371, but the provided issue objectives concern URL redirects, API request identification, licensing parameters, and account creation validation. They do not match the listing media authorization changes.

Full details: Linked Issues check

Explanation

Issue #371 requires coding changes for HTTPS theme-management URLs, API request user-agent identification, licensing query parameters and version handling, and account-creation validation conditions. The reviewed PR changes listing-media attachment and deletion authorization, image ownership helpers, media-selector gating, and related tests. The summary provides no evidence that any #371 requirement is implemented.

Resolution

Implement the coding requirements in issue #371, including the HTTPS URL, API user-agent, licensing, and account-validation changes, with automated tests. Alternatively, link this PR to the issue that defines the listing-media authorization work and handle #371 separately.

Full details: Out of Scope Changes check

Explanation

The PR changes AJAX listing-image attachment and deletion authorization, adds image ownership and capability checks, gates the media selector, and adds authorization tests. These changes support listing-media security, but they have no stated connection to issue #371's URL, API identification, licensing, or account-creation objectives. The changes are therefore outside the linked issue scope.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}