Skip to content

magento/magento2#33055: Validate filterable attribute input types on admin save - #41124

Open
swnsma wants to merge 1 commit into
magento:2.4-developfrom
swnsma:magento/magento2#33055
Open

magento/magento2#33055: Validate filterable attribute input types on admin save#41124
swnsma wants to merge 1 commit into
magento:2.4-developfrom
swnsma:magento/magento2#33055

Conversation

@swnsma

@swnsma swnsma commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Fixed Issues

  1. Fixes Missing backend validation for attribute types #33055

Manual testing scenarios

  1. In Admin, create a product attribute with Catalog Input Type = Text Field.
  2. In browser DevTools, remove disabled from Use in Layered Navigation, set it to Filterable (with results), and save.
  3. Verify save is rejected with: Can be used only with catalog input type Yes/No, Dropdown, Multiple Select and Price. The attribute must not be stored as filterable.
  4. Repeat for a Yes/No or Dropdown attribute with layered navigation enabled and verify it still saves.
  5. Via REST POST /V1/products/attributes, create a text attribute with is_filterable: true and verify 400. Create a select/boolean attribute with is_filterable: true and verify it still succeeds.

Questions or comments

REST already enforced this in ProductAttributeRepository::save() (after ACP2E-1422). Admin save never went through the repository ($model->save()), so the same invalid state could still be stored from the browser form by removing the HTML disabled attribute.

This change consolidates that behaviour so admin and API use the same allowlist (boolean, select, multiselect, price) via FilterableAllowedInputTypes. The admin controller shows the existing form error and does not persist; REST still throws InputException::invalidFieldValue.

The repository check is now an allowlist matching the admin form, not the previous denylist. Types such as media_image / gallery with is_filterable are rejected on the API as well.

Coverage: unit tests for the allowlist, admin save, and repository; integration tests for controller and repository create/update paths.

Reject Use in Layered Navigation for catalog input types the admin form
does not allow, using the same allowlist for admin save and REST.
@m2-assistant

m2-assistant Bot commented Aug 14, 2026

Copy link
Copy Markdown

Hi @swnsma. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.


For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P3 May be fixed according to the position in the backlog. Progress: pending review

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

Missing backend validation for attribute types

2 participants