[ObitoTM]

No description provided.

[github-actions]

🤖 AI PR Review

Risk level: low

Review

Summary

• This PR ensures validation errors from per-file restrictions are exposed at the media-field level so a validation message can be shown below the field. The main changes are in the SingleFileRestriction class (assets/src/frontend/media.field.restrictions/SingleFileRestriction.js) and the corresponding built assets under assets/build/frontend/*. The built files were regenerated accordingly.

What I checked

• Files changed:
  • assets/src/frontend/media.field.restrictions/SingleFileRestriction.js (source change)
  • assets/build/frontend/media.field.restrictions.js (bundle update)
  • assets/build/frontend/media.field.restrictions.asset.php (asset version bump)

Notes, potential issues and suggestions

1. Null checks around DOM nodes

   • Both the original and new code call callable.getFileNode(file.name).querySelector('.jet-form-builder-file-upload__file-invalid-marker') without guarding getFileNode(). If getFileNode() can ever return null, this will throw. Please add a guard before querying and before manipulating style/title, something like:
     const fileNode = callable.getFileNode(fileCurrent.name);
     if (! fileNode) { continue; }
     const marker = fileNode.querySelector(...);
     if (! marker) { continue; }
   • This prevents runtime errors on edge cases (custom file node structure, race conditions, or missing elements introduced by themes/plugins).

2. Choice of invalidRestriction value

   • The implementation sets this.invalidRestriction = results[0]?.rejected[0] ?? null; i.e. it picks the first rejected restriction of the first rejected file. This is fine as a pragmatic choice, but it should be deliberate: if multiple files have errors you choose the first one encountered. Consider adding a short comment documenting that behavior, or changing logic to pick the first non-empty human-friendly message across all rejected entries.

3. Clearing previous failure

   • Good: the PR clears this.invalidRestriction at the beginning of validatePromise (so the field-level message is cleared if the invalid file was removed). That resolves the bug described in the issue.

4. getMessage helper

   • Adding getMessage() to SingleFileRestriction (returning invalidRestriction?.getMessage() ?? '') is correct. Confirm that the code which renders the message at field level calls this method (I assume that is handled elsewhere). If not, ensure the field-level renderer uses singleFileRestriction.getMessage().

5. Tests

   • There are no unit/integration tests included. This touches client-side validation flow; please add tests (JS unit or e2e) that cover:
     • A single-file restriction failure shows the message at field level.
     • Removing the invalid file clears the field-level message.
     • Multiple files with one invalid file still triggers the field-level message.
   • If you have a visual regression or cypress/e2e test harness, an e2e test that uploads/removes files would be helpful.

6. Backward compatibility

   • This change is backwards compatible for public PHP APIs. It only affects client-side behavior of media field validation. No PHP sanitization/nonce/capability changes.

7. Performance and scalability

   • No new heavy operations are added. The code still uses the existing allRejected helper and Promise.allSettled. Looks fine for typical file counts; if sites accept hundreds of files at once this may be heavy, but that is an existing concern.

8. Build artifacts

   • The built asset and asset manifest were updated (version hash bumped). Ensure you ran the standard build pipeline for the repo and committed both source and build files (which you did). CI should verify this.

Minor style / maintainer notes

• Please add a newline at EOF in the built file (small diff showed previously missing newline).
• Consider a short inline comment where invalidRestriction is assigned to document the selection approach.

Conclusion

• The change looks correct and addresses the bug by surfacing per-file restriction messages at the media-field level and clearing them when files are removed. Please add DOM null-checks to avoid potential runtime errors, add tests covering removal and message clearing, and optionally document the choice of which rejected message is shown when multiple files fail.

Suggested changelog entry

- FIX: Display media field validation errors below the field (file/media field restrictions)