sadpandajoe opened a new pull request, #42819:
URL: https://github.com/apache/superset/pull/42819
### SUMMARY
Creating (or renaming) a Row Level Security rule with a name that already
exists was rejected only at DB flush time, by the `unique` constraint on
`RowLevelSecurityFilter.name`. That surfaced as a SQLAlchemy `IntegrityError`
whose string form is empty, so the API returned `422 {"message": "()"}` and the
UI rendered a toast with no reason in it:
```
An error occurred while creating rowlevelsecuritys: ()
```
The user is told the save failed but not why, even though the cause is a
simple, actionable name collision.
This adds an explicit name-uniqueness check to
`CreateRLSRuleCommand.validate()` and `UpdateRLSRuleCommand.validate()` that
raises a translatable `ValidationError` **before** the write, and surfaces it
from `post()`/`put()` as a `422` carrying the field message. The update path
excludes the rule being edited, so saving a rule without renaming it is not
treated as a collision.
New `RLSDAO.validate_uniqueness(name, rule_id=None)` mirrors the existing
pattern in `DatasetDAO`/`SemanticLayerDAO`.
After the change the response body is `{"message": {"name": ["A rule with
this name already exists."]}}` and the toast states the reason.
Note the `ValidationError` arm is deliberately ordered **after** the
existing `SubjectsNotFoundValidationError` /
`DatasourceNotFoundValidationError` arms, since those are `ValidationError`
subclasses and must keep their own responses. `RLSRuleNotFoundError` is a
`CommandException`, so the `put()` 404 path is unaffected.
### BEFORE/AFTER
Same scenario on `master` and on this branch: create a rule, then create a
second rule with the same name. The toast is the only user-visible difference:
| | Toast |
|---|---|
| Before | `An error occurred while creating rowlevelsecuritys: ()` |
| After | `An error occurred while creating rowlevelsecuritys: (name) A rule
with this name already exists.` |
The response body changes from `{"message": "()"}` to `{"message": {"name":
["A rule with this name already exists."]}}`; both are `422`.
Verified in a browser on both refs at 1440x900 and 375x667, in light and
dark themes, with the same result in each. Screen recordings of the
before/after runs will be attached to this description shortly.
### TESTING INSTRUCTIONS
1. Go to Settings → Row Level Security.
2. Create a rule named `my-rule` (Regular, any dataset, any role, any
clause). It saves.
3. Click **+ Rule** again and create another rule with the same name
`my-rule`.
4. Save. The toast now names the reason instead of showing empty parentheses.
5. Edit an existing rule and save it *without* changing the name — it still
saves, and is not rejected as a duplicate.
Automated coverage added:
- `tests/unit_tests/commands/security/rls_test.py` — create/update reject a
duplicate name, allow a unique name, and allow an unchanged name (asserting the
update check excludes the rule's own id).
- `tests/integration_tests/security/row_level_security_tests.py` — asserts
the actual HTTP body for the create and rename cases, and that an
unchanged-name save still returns 200.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Out of scope, pre-existing and unchanged by this PR: the frontend
`createErrorHandler` formatting still pluralizes the resource label as
`rowlevelsecuritys` and prefixes the field name, and the update path renders
the serialized field object rather than a flattened sentence. Those live in
`superset-frontend/src/views/CRUD/hooks.ts` and affect every CRUD resource, so
they are left for a separate change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
