fix(local_fields): permission is dependent of the linked resource

[PascalRepond]

• Partially addresses Allow seeing and editing local fields for any resource in the organisation #4091.

[coderabbitai]

Walkthrough

This PR enhances local field permissions by introducing a new AllowedByActionRestrictByLocalFieldParent class that validates both organisation and library-scoped access. The class checks that users possess the required OrganisationNeed and conditionally enforces LibraryNeed when the parent resource has a library_pid. LocalFieldPermissionPolicy now uses this constraint for create/update/delete operations. Test coverage is expanded with multiple scenarios validating permission behavior across different user-organisation-library contexts and role configurations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: local field permissions are now dependent on the linked resource instead of just the organisation.
Description check	✅ Passed	The description references issue #4091, providing context about the pull request's purpose, though it is brief.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

rero_ils/modules/local_fields/permissions.py (1)

57-64: ⚡ Quick win

Add a defensive guard to prevent None input to extracted_data_from_ref.

record.get("parent") returns None when the field is absent, and passing None to extracted_data_from_ref causes an AttributeError when attempting to call .get("$ref") on None. Additionally, if the parent record was deleted after the reference was created, record_class.get_record_by_pid() raises an unhandled exception, causing a 500 error instead of a permission denial. Wrapping the library check with a guard ensures the function only executes when the parent reference exists:

Proposed defensive guard

-        parent = extracted_data_from_ref(record.get("parent"), data="record")
-        if library_pid := getattr(parent, "library_pid", None):
-            if LibraryNeed(library_pid) not in g.identity.provides:
-                return []
+        parent_ref = record.get("parent")
+        if parent_ref:
+            parent = extracted_data_from_ref(parent_ref, data="record")
+            if library_pid := getattr(parent, "library_pid", None):
+                if LibraryNeed(library_pid) not in g.identity.provides:
+                    return []

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rero_ils/modules/local_fields/permissions.py` around lines 57 - 64, The code
calls extracted_data_from_ref(record.get("parent")) without guarding against a
missing parent and does not handle exceptions from resolving a deleted parent;
update the needs method to first assign parent_ref = record.get("parent") and
skip the parent/library checks (fall back to super().needs or return []) when
parent_ref is falsy, and wrap any call that resolves the parent (e.g.,
extracted_data_from_ref or record_class.get_record_by_pid) in a try/except so
that resolution errors return a permission-denied response (empty list) instead
of propagating an exception; keep checks using OrganisationNeed and LibraryNeed
and membership in g.identity.provides unchanged, only add the guard and
exception handling around the parent extraction/resolution.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@rero_ils/modules/local_fields/permissions.py`:
- Around line 57-64: The code calls
extracted_data_from_ref(record.get("parent")) without guarding against a missing
parent and does not handle exceptions from resolving a deleted parent; update
the needs method to first assign parent_ref = record.get("parent") and skip the
parent/library checks (fall back to super().needs or return []) when parent_ref
is falsy, and wrap any call that resolves the parent (e.g.,
extracted_data_from_ref or record_class.get_record_by_pid) in a try/except so
that resolution errors return a permission-denied response (empty list) instead
of propagating an exception; keep checks using OrganisationNeed and LibraryNeed
and membership in g.identity.provides unchanged, only add the guard and
exception handling around the parent extraction/resolution.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1a6c0f78-e8d8-4907-a107-86564750cac9

📥 Commits

Reviewing files that changed from the base of the PR and between 23a4356 and 0e15d59.

⛔ Files ignored due to path filters (1)

• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• rero_ils/modules/local_fields/permissions.py
• tests/api/local_fields/test_local_fields_permissions.py
• tests/fixtures/metadata.py

[coveralls]

coverage: 91.316% (+0.004%) from 91.312% — PascalRepond:rep-lofi-permissions into rero:staging

[PascalRepond]

Needs a PR to adapt rero-ils-ui