V2 benchfixes - #19
Open
dvaschenko wants to merge 3 commits into
Open
Conversation
Split the overloaded jmix-create-resource-role — which covered the access model, resource roles AND row-level roles under one misleading name — into: - jmix-role-based-access (new umbrella): the access model (resource roles are additive/no-deny; row-level roles are restrictive), security scope (UI/API), the mandatory ui.loginToUi login invariant, and DEBUG-logging diagnostics for silent access denials. Read before creating any role. - jmix-create-resource-role: resource roles only; adds minimum-policy mapping (an action like confirm/approve = READ+UPDATE, not ALL), a gotcha against hand-rolling role checks via a raw GrantedAuthority (use AccessManager with CrudEntityContext/EntityAttributeContext), and a non-admin login self-check. - jmix-create-row-level-role (new): @RowLevelRole with JPQL and predicate policies, incl. per-row write gating — previously buried under the resource-role name and hard to discover. Why: when driving an LLM agent to build apps on the pack, domain-only roles repeatedly could not log into the UI (generic "Login failed") because ui.loginToUi / the built-in ui-minimal role was never mentioned — the single most commonly missed defect. The umbrella now states it as a mandatory invariant with a self-check. Identifiers verified against Jmix 2.8.2 jars. AGENTS.md skill routing and .studio/skills-manifest.json updated accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…view Each item is a general framework-idiom gap or an inaccurate claim, verified against Jmix 2.8.2 jars/sources: - jmix-create-list-view: a column on a reference attribute needs the reference added to the fetch plan (this is an N+1/loading concern — defer the exact rules to jmix-configure-fetch-plan; it is NOT a "reference throws unfetched" crash, which is the local-attribute-in-a-partial-plan case). list_create / list_edit require an existing <Entity>.detail view (else NoSuchViewException on first click). Added a menu.xml <item> registration example. - jmix-create-entity: a plain @manytoone needs an explicit fetch = LAZY (JPA defaults to EAGER); @column(unique = true) alone does not create the constraint (Liquibase builds the schema). - jmix-create-liquibase-changelog: added an <addUniqueConstraint> example; the composition-child FK cascade now depends on the delete mode — soft-deleted entities cascade at the app layer (@onDelete, no DB onDelete), hard-deleted entities MUST declare onDelete="CASCADE" on the FK or the delete FK-violates (soft delete is NOT global — it applies only to Soft-Delete-trait entities). - jmix-create-service: use .optional() for finders that may not match; added a gotcha that calling save() twice on one reference without reassigning the returned instance throws OptimisticLockException (stale @Version). - jmix-create-detail-view: corrected the entityComboBox-bound-to-enum failure wording (it requires an entity Range, not an enumeration). Why: surfaced by repeatedly building the same CRUD app with an LLM agent on the pack — these are the defects the agent hit that compile and pass a green test but fail (or degrade) at runtime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KartnDev
approved these changes
Jul 13, 2026
| ## Row-Level roles | ||
| ## Row-level roles | ||
|
|
||
| Row-level roles are a separate first-class concept from resource roles: a resource |
Contributor
There was a problem hiding this comment.
is it fine deletion - just refactor im ok
Fedoseew
reviewed
Jul 13, 2026
| - `@PredicateRowLevelPolicy(entityClass = ..., actions = {...})` is tested per | ||
| instance and covers ANY of `READ`, `CREATE`, `UPDATE`, `DELETE` — this is how you | ||
| gate WRITE operations per row (e.g. read all rows but UPDATE/DELETE only owned | ||
| ones). The method returns a `RowLevelPredicate` / `RowLevelBiPredicate`; use it |
Contributor
There was a problem hiding this comment.
Probably we should mention here about performance issues for this mechanism?
Fedoseew
reviewed
Jul 13, 2026
| .query("select e from Customer e where e.active = true") | ||
| .list(); | ||
|
|
||
| // Finder that may not match: .one() throws when there is no match (or more than one). |
Contributor
There was a problem hiding this comment.
do we need this comment here?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Предложения по фиксам в скиллы по результатам прогона на GLM 5.2