feat(ebios_rm): make third-party entity and ebiosrm's stakeholders relation optional#3946
Open
martinzerty wants to merge 27 commits into
Open
feat(ebios_rm): make third-party entity and ebiosrm's stakeholders relation optional#3946martinzerty wants to merge 27 commits into
martinzerty wants to merge 27 commits into
Conversation
…RM study's stakeholder
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces Stakeholder.entity FK with a free-text ChangesStakeholder migration and UI updates
Sequence Diagram(s)sequenceDiagram
participant Importer as DataWizard Import
participant Backend as Django Views
participant DB as Database
Importer->>Backend: POST import row (entity_name or entity)
Backend->>DB: SELECT Stakeholder WHERE entity_name ILIKE value
alt found
DB-->>Backend: existing Stakeholder
Backend-->>Importer: associate existing stakeholder
else not found
Backend->>DB: INSERT Stakeholder(entity_name=..., third_party_entity=entity_if_any)
DB-->>Backend: created Stakeholder
Backend-->>Importer: created stakeholder
end
sequenceDiagram
participant User
participant Frontend
participant API as Backend API
participant DB as Database
User->>Frontend: open stakeholder modal, choose third_party_entity
Frontend->>Frontend: set entity_name from selected third_party_entity (if new)
User->>Frontend: submit form with entity_name + third_party_entity
Frontend->>API: POST /stakeholders {entity_name, third_party_entity, ...}
API->>DB: create/update Stakeholder with entity_name and third_party_entity
DB-->>API: persistence result
API-->>Frontend: 201/200 response
Frontend->>User: close modal / update table
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte (2)
131-143: Add adata-testidto the new entity button for E2E consistency.The existing stakeholder button exposes
data-testid="add-button". The new entity button has no stable selector, which will force tests to rely on icon classes or title text. Consider adding something likedata-testid="add-entity-button"(and optionally renaming the existing one toadd-stakeholder-button) so each action can be targeted unambiguously.<button class="inline-block p-3 btn-mini-primary w-12 focus:relative border-r" title={safeTranslate('add-' + data.entityModel.localName)} + data-testid="add-entity-button" onclick={modalCreateEntityForm} ><i class="fa-solid fa-building"></i> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte around lines 131 - 143, Add a stable test selector to the new entity button by adding a data-testid attribute (e.g., data-testid="add-entity-button") to the <button> that calls modalCreateEntityForm so E2E tests can target it reliably; optionally rename the existing button's data-testid (currently data-testid="add-button" on the button that calls modalCreateForm) to something like data-testid="add-stakeholder-button" for unambiguous selectors.
29-61: Consider extracting a shared helper to avoid near-duplication of the two modal openers.
modalCreateFormandmodalCreateEntityFormare structurally identical except forform,formAction, andmodel. Parameterizing a single helper keeps future changes (titles, props, store usage) in one place.♻️ Proposed refactor
- function modalCreateForm(): void { - let modalComponent: ModalComponent = { - ref: CreateModal, - props: { - form: data.createForm, - model: data.model - } - }; - let modal: ModalSettings = { - type: 'component', - component: modalComponent, - // Data - title: safeTranslate('add-' + data.model.localName) - }; - modalStore.trigger(modal); - } - - function modalCreateEntityForm(): void { - let modalComponent: ModalComponent = { - ref: CreateModal, - props: { - form: data.entityCreateForm, - formAction: '?/createEntity', - model: data.entityModel - } - }; - let modal: ModalSettings = { - type: 'component', - component: modalComponent, - title: safeTranslate('add-' + data.entityModel.localName) - }; - modalStore.trigger(modal); - } + function openCreateModal( + form: unknown, + model: { localName: string }, + formAction?: string + ): void { + const modalComponent: ModalComponent = { + ref: CreateModal, + props: { form, model, ...(formAction ? { formAction } : {}) } + }; + const modal: ModalSettings = { + type: 'component', + component: modalComponent, + title: safeTranslate('add-' + model.localName) + }; + modalStore.trigger(modal); + } + + const modalCreateForm = () => openCreateModal(data.createForm, data.model); + const modalCreateEntityForm = () => + openCreateModal(data.entityCreateForm, data.entityModel, '?/createEntity');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte around lines 29 - 61, Extract a shared helper that builds and triggers the modal to remove duplication between modalCreateForm and modalCreateEntityForm: create a function (e.g., openModalWithComponent) that accepts parameters for form, model, optional formAction and titleKey, constructs the ModalComponent (ref: CreateModal, props: { form, formAction?, model }) and ModalSettings (type: 'component', component, title: safeTranslate(titleKey)), then calls modalStore.trigger(modal); replace modalCreateForm and modalCreateEntityForm to call this helper with data.createForm/data.model and data.entityCreateForm/data.entityModel plus '?/createEntity' for formAction and appropriate title keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte:
- Around line 131-143: Add a stable test selector to the new entity button by
adding a data-testid attribute (e.g., data-testid="add-entity-button") to the
<button> that calls modalCreateEntityForm so E2E tests can target it reliably;
optionally rename the existing button's data-testid (currently
data-testid="add-button" on the button that calls modalCreateForm) to something
like data-testid="add-stakeholder-button" for unambiguous selectors.
- Around line 29-61: Extract a shared helper that builds and triggers the modal
to remove duplication between modalCreateForm and modalCreateEntityForm: create
a function (e.g., openModalWithComponent) that accepts parameters for form,
model, optional formAction and titleKey, constructs the ModalComponent (ref:
CreateModal, props: { form, formAction?, model }) and ModalSettings (type:
'component', component, title: safeTranslate(titleKey)), then calls
modalStore.trigger(modal); replace modalCreateForm and modalCreateEntityForm to
call this helper with data.createForm/data.model and
data.entityCreateForm/data.entityModel plus '?/createEntity' for formAction and
appropriate title keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ecc3c6d-e771-4629-844d-d309f2367670
📒 Files selected for processing (2)
frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte:
- Around line 131-137: The icon-only <button> (the element with class
"inline-block p-3 btn-mini-primary w-12..." and onclick handler
modalCreateEntityForm and data-testid "add-entity-button") needs an explicit
accessible name and to be safe in forms: add an aria-label using the same
safeTranslate('add-' + data.entityModel.localName) expression (e.g.,
aria-label={safeTranslate('add-' + data.entityModel.localName)}) and add
type="button" to prevent accidental form submission.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f11357ee-88c3-4c0f-8502-db634dc2e3ae
📒 Files selected for processing (2)
frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte
martinzerty
added
the
db-migration
martinzerty
requested review from
Mohamed-Hacene,
ab-smith,
eric-intuitem and
nas-tabchiche
martinzerty
changed the title
martinzerty
changed the title
…selected category and metrics
martinzerty
changed the title
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/ebios_rm/serializers.py (1)
259-272:⚠️ Potential issue | 🔴 Critical
entityfield doesn't exist on theStakeholdermodel — export will raiseAttributeError.The model now defines
entity_name(TextField) andthird_party_entity(FK); the legacyentityattribute is gone. Declaringentity = serializers.CharField(read_only=True)withoutsource="entity_name"causes DRF togetattr(instance, "entity")during serialization, which will fail on any Stakeholder export via the domain backup/restore serializer.Meta.fields at line 271 and the model's
fields_to_checklist (models.py:650) also carry the same stale name. Align all three toentity_name(matchingStakeholderReadSerializerat line 236):Proposed fixes
backend/ebios_rm/serializers.py (lines 259 & 271):
class StakeholderImportExportSerializer(BaseModelSerializer): folder = HashSlugRelatedField(slug_field="pk", read_only=True) ebios_rm_study = HashSlugRelatedField(slug_field="pk", read_only=True) - entity = serializers.CharField(read_only=True) + entity_name = serializers.CharField(read_only=True) third_party_entity = HashSlugRelatedField(slug_field="pk", read_only=True) applied_controls = HashSlugRelatedField(slug_field="pk", read_only=True, many=True) category = serializers.SlugRelatedField(slug_field="name", read_only=True) class Meta: model = Stakeholder fields = [ "created_at", "updated_at", "folder", "ebios_rm_study", - "entity", + "entity_name", "third_party_entity",backend/ebios_rm/models.py (line 650):
- fields_to_check = ["ebios_rm_study", "entity", "category"] + fields_to_check = ["ebios_rm_study", "entity_name", "category"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/ebios_rm/serializers.py` around lines 259 - 272, The serializer declares entity = serializers.CharField(read_only=True) which no longer exists on the Stakeholder model; update the export/backup serializer to reference the current field name entity_name (or set source="entity_name") and remove the stale entity reference from Meta.fields so it contains "entity_name" instead; also update the model's fields_to_check list in Stakeholder (models.py, fields_to_check) to replace "entity" with "entity_name" so the backup/restore and StakeholderReadSerializer remain consistent with the model.backend/ebios_rm/models.py (1)
650-650:⚠️ Potential issue | 🔴 CriticalReplace stale field reference in
fields_to_check.The field
"entity"does not exist onStakeholder; it was renamed toentity_name. Thefields_to_checklist is used bybackend/core/base_models.pyandbackend/data_wizard/views.pyto perform duplicate detection and validation, and both will raiseAttributeErrorwhen attempting to access the non-existent field.Suggested fix
- fields_to_check = ["ebios_rm_study", "entity", "category"] + fields_to_check = ["ebios_rm_study", "entity_name", "category"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/ebios_rm/models.py` at line 650, The fields_to_check list in Stakeholder's model currently references a removed attribute "entity" which causes AttributeError during duplicate detection; update the list (fields_to_check in backend/ebios_rm/models.py) to use the renamed attribute "entity_name" instead of "entity" so that code paths in backend/core/base_models.py and backend/data_wizard/views.py can access the correct field; verify there are no other references to "entity" in the Stakeholder class or its validation/duplication helpers and run tests for duplicate detection to confirm the fix.
♻️ Duplicate comments (1)
frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte (1)
112-118:⚠️ Potential issue | 🟡 MinorAdd explicit accessible name and button type on the icon-only action button.
At Line 112, the icon-only button still relies on
titleonly. Addaria-labelandtype="button".♿ Proposed fix
<button + type="button" class="inline-block p-3 btn-mini-primary w-12 focus:relative" data-testid="add-button" title={safeTranslate('add-' + data.model.localName)} + aria-label={safeTranslate('add-' + data.model.localName)} onclick={modalCreateForm} ><i class="fa-solid fa-file-circle-plus"></i> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte around lines 112 - 118, The icon-only action button using onclick={modalCreateForm} and title={safeTranslate('add-' + data.model.localName)} needs an explicit accessible name and explicit button type: add aria-label with the same translated string (aria-label={safeTranslate('add-' + data.model.localName)}) and add type="button" to the button element (the element with data-testid="add-button" and onclick handler modalCreateForm) so it’s recognized correctly by assistive tech and doesn’t submit forms accidentally.
🧹 Nitpick comments (6)
frontend/messages/hr.json (1)
805-806: Duplicate Croatian translation forentityandentity_name.Both keys now resolve to
"Entitet", which will make the two fields visually indistinguishable in the UI (e.g., in forms/tables where a stakeholder has both its own entity name and an optional third-party entity reference nearby). If the English source differentiates them (e.g., "Entity" vs. "Entity name"), consider mirroring the distinction here (e.g.,"entity_name": "Naziv entiteta") to avoid ambiguity for Croatian users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/hr.json` around lines 805 - 806, The Croatian translations for the keys "entity" and "entity_name" are identical; update the "entity_name" value to a distinct Croatian phrase (e.g., "Naziv entiteta") so the UI differentiates between the general "entity" ("Entitet") and the specific "entity_name" field; modify the "entity_name" entry accordingly in frontend/messages/hr.json.frontend/messages/hu.json (2)
751-751: Disambiguateentity_namefromentityin Hungarian UI text.Line 751 currently mirrors
entity("Vállalat"), so users may not recognize this as the stakeholder’s own entity name field. Consider"Vállalat neve".✏️ Suggested translation tweak
- "entity_name": "Vállalat", + "entity_name": "Vállalat neve",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/hu.json` at line 751, The Hungarian translation for the key "entity_name" in frontend/messages/hu.json is ambiguous (currently "Vállalat") and should be changed to "Vállalat neve" to clearly indicate the stakeholder's own entity name field; update the value for the "entity_name" key only (leave the "entity" key as-is) so UI text disambiguates between the entity label and the entity name field.
759-760: Prefer consistent terminology (vállalat) for entity-related labels.Lines 759-760 use
entitás, while this locale generally usesVállalatforentity. Keeping one term improves form consistency.✏️ Suggested consistency tweak
- "thirdPartyEntity": "Harmadik fél entitás", - "thirdPartyEntityHelpText": "Az a harmadik fél entitás, amelyhez az érdekelt felet kapcsolni szeretné.", + "thirdPartyEntity": "Harmadik fél vállalat", + "thirdPartyEntityHelpText": "Az a harmadik fél vállalat, amelyhez az érdekelt felet kapcsolni szeretné.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/hu.json` around lines 759 - 760, The locale uses inconsistent terminology: the keys "thirdPartyEntity" and "thirdPartyEntityHelpText" use "entitás" but the project prefers "Vállalat" for entity-related labels; update the Hungarian translations for those keys to use "Vállalat" (e.g., change "Harmadik fél entitás" and its help text to use "Harmadik fél Vállalat" or equivalent phrasing) so the terminology matches the rest of the locale.frontend/messages/lt.json (1)
1150-1150: Makeentity_nameexplicit to avoid label ambiguity.Line 1150 uses the same visible text as
entity("Subjektas"), which can make the new free-text field unclear in forms. Prefer a name-specific label (e.g.,"Subjekto pavadinimas").✏️ Suggested translation tweak
- "entity_name": "Subjektas", + "entity_name": "Subjekto pavadinimas",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/lt.json` at line 1150, The "entity_name" translation value is ambiguous because it duplicates "entity" ("Subjektas"); update the "entity_name" key's Lithuanian string to a more specific label such as "Subjekto pavadinimas" so the free-text field is clearly distinguished in forms—locate the "entity_name" entry in frontend/messages/lt.json and replace its value accordingly.frontend/src/lib/utils/table.ts (1)
2260-2260:entity_namefilter is still tied to theentitiesendpoint.
entity_nameis now a standalone stakeholder field, so filtering viaENTITY_FILTER(optionsEndpoint: 'entities') can hide stakeholder names that are not backed by an Entity record.♻️ Suggested adjustment
+export const STAKEHOLDER_ENTITY_NAME_FILTER: ListViewFilterConfig = { + component: AutocompleteSelect, + props: { + label: 'entity_name', + optionsEndpoint: 'stakeholders/entity_name', + optionsLabelField: 'value', + optionsValueField: 'value', + multiple: true + } +}; stakeholders: { ... filters: { is_selected: IS_SELECTED_FILTER, - entity_name: ENTITY_FILTER, + entity_name: STAKEHOLDER_ENTITY_NAME_FILTER, category: STAKEHOLDER_CATEGORY_FILTER } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/utils/table.ts` at line 2260, The filter for key entity_name is still configured to use the entities endpoint via the ENTITY_FILTER optionsEndpoint, which hides stakeholder names not backed by Entity records; update the ENTITY_FILTER definition so entity_name is not tied to optionsEndpoint: 'entities' — either remove the optionsEndpoint or change it to the stakeholder source (e.g., 'stakeholders') or wire it to the standalone stakeholder field provider so the filter pulls from the actual stakeholder list instead of the entities endpoint (locate the ENTITY_FILTER constant and its optionsEndpoint property in table.ts and adjust accordingly).frontend/src/lib/utils/schemas.ts (1)
1188-1189: Consider cross-field validation: stakeholder can be created with no entity identification.With
entity_name: z.string()(which accepts"") andthird_party_entityoptional/nullable, a stakeholder can pass validation with neither a meaningfulentity_namenor athird_party_entity— the backend model mirrors this (TextFielddefault=""+ nullable FK), so nothing will reject it downstream either. If the PR's intent is "use entity_name when no third-party entity is associated", it's worth enforcing that at least one of the two is provided at the form layer.Also note
third_party_entityis declared asz.string()without.uuid(), which is inconsistent with most other FK id fields in this file (e.g.,compliance_assessments: z.string().uuid().optional().nullable()).♻️ Suggested change
- entity_name: z.string(), - third_party_entity: z.string().optional().nullable(), + entity_name: z.string().default(''), + third_party_entity: z.string().uuid().optional().nullable(), current_dependency: z.number().min(0).max(4).default(0).optional(),Then add a
.refine(...)on the object to require at least one identifier, e.g.:export const StakeholderSchema = z .object({ /* ...fields... */ }) .refine((v) => (v.entity_name && v.entity_name.trim() !== '') || !!v.third_party_entity, { message: 'Either an entity name or a third-party entity must be provided', path: ['entity_name'] });Please confirm whether the product requirement allows a stakeholder with no entity information at all, or whether the form/UI is expected to enforce the "one of the two" rule. If the UI handles it, a schema-level guard is still useful as a safety net.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/utils/schemas.ts` around lines 1188 - 1189, The Stakeholder schema currently allows neither a meaningful entity identifier nor a valid FK (entity_name accepts empty string and third_party_entity is optional/nullable and not validated as a UUID); update the StakeholderSchema to (1) change third_party_entity to use .string().uuid().optional().nullable() like other FK fields and (2) add an object-level .refine(...) on StakeholderSchema that requires either entity_name.trim() !== '' or a non-empty third_party_entity (UUID), providing a clear message (e.g., "Either an entity name or a third-party entity must be provided") and set the path to ['entity_name'] for UX.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/data_wizard/views.py`:
- Around line 4925-4926: Several places still use the stale "entity" field
causing ORM mismatches: replace filters using "entity__iexact" with
"entity_name__iexact" and update object creation/upserts that use "entity=..."
to use "entity_name=..." instead; search for occurrences around the
functions/methods that perform Stakeholder lookups and creations (the blocks
around the lines referencing entity__iexact and create/update calls) and update
them to the current Stakeholder schema (entity_name and third_party_entity)
while leaving third_party_entity usage unchanged so conflict matching and ORM
operations succeed.
In
`@backend/ebios_rm/migrations/0025_remove_stakeholder_entity_stakeholder_entity_name_and_more.py`:
- Around line 13-35: The migration currently drops Stakeholder.entity without
preserving existing values; add a RunPython data migration step executed before
the RemoveField that loads the historical Stakeholder and Entity models,
iterates rows and sets stakeholder.third_party_entity_id = stakeholder.entity_id
and stakeholder.entity_name = stakeholder.entity.name (handling nulls), then
saves; reference the migration operations for model_name="stakeholder" and
fields "entity", "third_party_entity", and "entity_name" and ensure the
RunPython forwards the copy logic and includes a no-op reverse function.
In `@backend/ebios_rm/models.py`:
- Around line 576-585: Existing Stakeholder rows will be wiped because the
migration removes the entity FK before preserving its data; add a RunPython
backfill migration (e.g., backfill_entity_name) inserted before RemoveField that
uses apps.get_model("ebios_rm", "Stakeholder"), iterates with
select_related("entity"), sets sh.entity_name = sh.entity.name if sh.entity_id
else "" and sh.third_party_entity_id = sh.entity_id, then saves with
update_fields=["entity_name","third_party_entity"]; reference the new
entity_name and third_party_entity fields and use
migrations.RunPython(backfill_entity_name, migrations.RunPython.noop).
In `@backend/ebios_rm/views.py`:
- Line 503: The XLSX export currently emits only "entity_name": sh.entity_name
which loses the stakeholder name when entity_name is empty but
third_party_entity is set; update the export to fall back to the linked
third-party's name by changing the value to something like: "entity_name":
sh.entity_name if sh.entity_name else (sh.third_party_entity.name if
sh.third_party_entity else ""), referencing the stakeholder object sh and its
third_party_entity relation so the sheet preserves identity when only
third_party_entity is populated.
- Line 936: The report_data code still uses the old relation path
order_by("entity__name") which will raise a FieldError now that filtering/fields
use "entity_name"; update report_data to use the new flat field name (e.g.,
replace order_by("entity__name") with order_by("entity_name") and any other
references to entity__name) so all sorting/filtering uses the `entity_name`
field consistently (look for uses in the report_data function and related
queryset constructions).
In `@frontend/messages/de.json`:
- Line 720: The translation for the key entity_name is too generic and may
duplicate the existing entity label; update the value for entity_name to a more
specific German label such as "Entitätsname" or "Name der Entität" so users can
distinguish the fields—locate the entity_name entry in the
frontend/messages/de.json and replace "Entität" with the chosen more specific
string.
In `@frontend/messages/fr.json`:
- Line 1050: The French translation for the key "entity_name" is ambiguous
because it duplicates the existing "entity" label; update the value of the
"entity_name" key to a clearer phrase such as "Nom de l'entité" so the free-text
field is distinguished from the "entity" label (change the value for
"entity_name" from "Entité" to "Nom de l'entité").
In `@frontend/messages/it.json`:
- Around line 737-738: The Italian translation uses plural wording for the keys
thirdPartyEntity and thirdPartyEntityHelpText; change both to singular phrasing
— update thirdPartyEntity from "Entità di terze parti" to "Entità di terza
parte" and update thirdPartyEntityHelpText from "L'entità di terze parti a cui
si desidera collegare la parte interessata." to "L'entità di terza parte a cui
si desidera collegare la parte interessata." so the label and help text match
the singular key.
In `@frontend/messages/nl.json`:
- Around line 728-729: The label for the third-party entity is ambiguous; update
the "thirdPartyEntity" translation value to a more explicit phrase (e.g.,
include "organisatie" or "entiteit" like "Derde partij (organisatie)" or "Derde
partij (entiteit)") and adjust the "thirdPartyEntityHelpText" if needed so both
keys ("thirdPartyEntity" and "thirdPartyEntityHelpText") consistently convey
that this field refers to an external organization/entity the stakeholder will
be linked to.
In `@frontend/src/lib/components/Forms/ModelForm/StakeholderForm.svelte`:
- Around line 93-102: The effect that auto-fills on third-party selection is
writing to the wrong field: it assigns to $formData.entity but the form schema
and input use entity_name; update the $effect handler that reads
selectedEntityOption and, when context !== 'edit', set $formData.entity_name =
entity.name ?? entity.str ?? $formData.entity_name (not $formData.entity). Also
trim the leading comment (the $effect comment referencing "relationship
category, and default assessment fields") so it only describes the current
behavior (auto-filling the entity_name) and leave the rest of the logic
unchanged.
---
Outside diff comments:
In `@backend/ebios_rm/models.py`:
- Line 650: The fields_to_check list in Stakeholder's model currently references
a removed attribute "entity" which causes AttributeError during duplicate
detection; update the list (fields_to_check in backend/ebios_rm/models.py) to
use the renamed attribute "entity_name" instead of "entity" so that code paths
in backend/core/base_models.py and backend/data_wizard/views.py can access the
correct field; verify there are no other references to "entity" in the
Stakeholder class or its validation/duplication helpers and run tests for
duplicate detection to confirm the fix.
In `@backend/ebios_rm/serializers.py`:
- Around line 259-272: The serializer declares entity =
serializers.CharField(read_only=True) which no longer exists on the Stakeholder
model; update the export/backup serializer to reference the current field name
entity_name (or set source="entity_name") and remove the stale entity reference
from Meta.fields so it contains "entity_name" instead; also update the model's
fields_to_check list in Stakeholder (models.py, fields_to_check) to replace
"entity" with "entity_name" so the backup/restore and StakeholderReadSerializer
remain consistent with the model.
---
Duplicate comments:
In
`@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte:
- Around line 112-118: The icon-only action button using
onclick={modalCreateForm} and title={safeTranslate('add-' +
data.model.localName)} needs an explicit accessible name and explicit button
type: add aria-label with the same translated string
(aria-label={safeTranslate('add-' + data.model.localName)}) and add
type="button" to the button element (the element with data-testid="add-button"
and onclick handler modalCreateForm) so it’s recognized correctly by assistive
tech and doesn’t submit forms accidentally.
---
Nitpick comments:
In `@frontend/messages/hr.json`:
- Around line 805-806: The Croatian translations for the keys "entity" and
"entity_name" are identical; update the "entity_name" value to a distinct
Croatian phrase (e.g., "Naziv entiteta") so the UI differentiates between the
general "entity" ("Entitet") and the specific "entity_name" field; modify the
"entity_name" entry accordingly in frontend/messages/hr.json.
In `@frontend/messages/hu.json`:
- Line 751: The Hungarian translation for the key "entity_name" in
frontend/messages/hu.json is ambiguous (currently "Vállalat") and should be
changed to "Vállalat neve" to clearly indicate the stakeholder's own entity name
field; update the value for the "entity_name" key only (leave the "entity" key
as-is) so UI text disambiguates between the entity label and the entity name
field.
- Around line 759-760: The locale uses inconsistent terminology: the keys
"thirdPartyEntity" and "thirdPartyEntityHelpText" use "entitás" but the project
prefers "Vállalat" for entity-related labels; update the Hungarian translations
for those keys to use "Vállalat" (e.g., change "Harmadik fél entitás" and its
help text to use "Harmadik fél Vállalat" or equivalent phrasing) so the
terminology matches the rest of the locale.
In `@frontend/messages/lt.json`:
- Line 1150: The "entity_name" translation value is ambiguous because it
duplicates "entity" ("Subjektas"); update the "entity_name" key's Lithuanian
string to a more specific label such as "Subjekto pavadinimas" so the free-text
field is clearly distinguished in forms—locate the "entity_name" entry in
frontend/messages/lt.json and replace its value accordingly.
In `@frontend/src/lib/utils/schemas.ts`:
- Around line 1188-1189: The Stakeholder schema currently allows neither a
meaningful entity identifier nor a valid FK (entity_name accepts empty string
and third_party_entity is optional/nullable and not validated as a UUID); update
the StakeholderSchema to (1) change third_party_entity to use
.string().uuid().optional().nullable() like other FK fields and (2) add an
object-level .refine(...) on StakeholderSchema that requires either
entity_name.trim() !== '' or a non-empty third_party_entity (UUID), providing a
clear message (e.g., "Either an entity name or a third-party entity must be
provided") and set the path to ['entity_name'] for UX.
In `@frontend/src/lib/utils/table.ts`:
- Line 2260: The filter for key entity_name is still configured to use the
entities endpoint via the ENTITY_FILTER optionsEndpoint, which hides stakeholder
names not backed by Entity records; update the ENTITY_FILTER definition so
entity_name is not tied to optionsEndpoint: 'entities' — either remove the
optionsEndpoint or change it to the stakeholder source (e.g., 'stakeholders') or
wire it to the standalone stakeholder field provider so the filter pulls from
the actual stakeholder list instead of the entities endpoint (locate the
ENTITY_FILTER constant and its optionsEndpoint property in table.ts and adjust
accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4843613-91ae-41b7-bb8e-38550e92fd9d
📒 Files selected for processing (37)
backend/data_wizard/views.pybackend/ebios_rm/helpers.pybackend/ebios_rm/migrations/0025_remove_stakeholder_entity_stakeholder_entity_name_and_more.pybackend/ebios_rm/models.pybackend/ebios_rm/serializers.pybackend/ebios_rm/tests/test_stakeholder.pybackend/ebios_rm/views.pyfrontend/messages/ar.jsonfrontend/messages/cs.jsonfrontend/messages/da.jsonfrontend/messages/de.jsonfrontend/messages/el.jsonfrontend/messages/en.jsonfrontend/messages/es.jsonfrontend/messages/fr.jsonfrontend/messages/hi.jsonfrontend/messages/hr.jsonfrontend/messages/hu.jsonfrontend/messages/id.jsonfrontend/messages/it.jsonfrontend/messages/ko.jsonfrontend/messages/lt.jsonfrontend/messages/nl.jsonfrontend/messages/pl.jsonfrontend/messages/pt.jsonfrontend/messages/ro.jsonfrontend/messages/sv.jsonfrontend/messages/tr.jsonfrontend/messages/uk.jsonfrontend/messages/ur.jsonfrontend/messages/zh.jsonfrontend/src/lib/components/Forms/ModelForm/StakeholderForm.sveltefrontend/src/lib/utils/schemas.tsfrontend/src/lib/utils/table.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.sveltefrontend/tests/functional/detailed/ebios-rm.test.ts
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/ebios_rm/serializers.py (1)
259-272:⚠️ Potential issue | 🟠 MajorDon’t make
entity_nameexport-only in the import/export serializer.
entity_nameis now the stakeholder’s standalone identifier, butread_only=Trueguarantees incoming payloads ignore it. That makes the import half of this serializer unable to recreate stakeholders that have nothird_party_entity, because they fall back to the model default empty string.🐛 Proposed fix
- entity_name = serializers.CharField(read_only=True) + entity_name = serializers.CharField(required=False, allow_blank=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/ebios_rm/serializers.py` around lines 259 - 272, The entity_name field is marked read_only which prevents imports from setting the stakeholder's standalone identifier; change the field declaration in the Stakeholder serializer from entity_name = serializers.CharField(read_only=True) to a writable CharField (e.g. entity_name = serializers.CharField(required=True, allow_blank=False) or at minimum entity_name = serializers.CharField()) so incoming payloads can populate the identifier during import and recreate stakeholders without third_party_entity.
♻️ Duplicate comments (6)
frontend/messages/fr.json (1)
1050-1050:⚠️ Potential issue | 🟡 MinorClarify
entity_nameto distinguish it from linked entities.
"entity_name": "Entité"is still ambiguous and overlaps with"entity". Use a clearer label like"Nom de l'entité"for the free-text field.Proposed fix
- "entity_name": "Entité", + "entity_name": "Nom de l'entité",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/fr.json` at line 1050, The translation key "entity_name" has an ambiguous French label "Entité" that conflicts with the existing "entity" label; update the value for the "entity_name" key to a clearer phrase such as "Nom de l'entité" so the free-text field is distinguished from linked/entity reference labels (modify the "entity_name" entry in frontend/messages/fr.json accordingly).frontend/messages/nl.json (1)
728-729:⚠️ Potential issue | 🟡 MinorMake
thirdPartyEntityexplicit as an entity/organization label.
"Derde partij"is still generic for an entity field and can be confused with category/type labeling.✏️ Suggested wording update
- "thirdPartyEntity": "Derde partij", - "thirdPartyEntityHelpText": "De derde partij waaraan u de stakeholder wilt koppelen.", + "thirdPartyEntity": "Derde partij-entiteit", + "thirdPartyEntityHelpText": "De derde partij-entiteit waaraan u de stakeholder wilt koppelen.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/nl.json` around lines 728 - 729, Update the Dutch label for the "thirdPartyEntity" key to be explicit that it refers to an entity/organization (e.g., change "Derde partij" to "Entiteit/organisatie" or similar), and ensure the related "thirdPartyEntityHelpText" still clarifies that this is the external entity you want to link the stakeholder to; modify the values for the "thirdPartyEntity" and, if needed for clarity, "thirdPartyEntityHelpText" entries in frontend/messages/nl.json accordingly.frontend/messages/de.json (1)
720-720:⚠️ Potential issue | 🟡 Minor
entity_nameis still ambiguous withentity.
entity_namecurrently duplicatesentity("Entität"), so users may not distinguish the free-text name field from the entity selector.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/de.json` at line 720, The "entity_name" translation duplicates "entity" ("Entität") and is ambiguous; update the "entity_name" key in frontend/messages/de.json to a clearer German label (e.g., "Entitätsname" or "Name der Entität") to distinguish the free-text name field from the entity selector, and verify the UI labels that reference "entity" and "entity_name" (keys "entity" and "entity_name") remain distinct and consistent across the app.frontend/messages/it.json (1)
737-738:⚠️ Potential issue | 🟡 MinorKeep
thirdPartyEntitysingular.The current Italian wording still reads plural and doesn't match the singular key.
Based on learnings: in `frontend/messages/it.json`, `thirdPartyEntity` should use singular wording.✏️ Suggested fix
- "thirdPartyEntity": "Entità di terze parti", - "thirdPartyEntityHelpText": "L'entità di terze parti a cui si desidera collegare la parte interessata.", + "thirdPartyEntity": "Entità di terza parte", + "thirdPartyEntityHelpText": "L'entità di terza parte a cui si desidera collegare la parte interessata.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/messages/it.json` around lines 737 - 738, The Italian translation for the key thirdPartyEntity is currently plural; update the value for "thirdPartyEntity" from "Entità di terze parti" to the singular wording "Entità di terza parte" and also make the help text consistent by changing "thirdPartyEntityHelpText" from "L'entità di terze parti a cui si desidera collegare la parte interessata." to "L'entità di terza parte a cui si desidera collegare la parte interessata." to keep both labels singular and aligned with the key.frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte (1)
112-118:⚠️ Potential issue | 🟡 MinorRestore button semantics for the icon-only action.
The control still needs an explicit accessible name and
type="button"so it works cleanly with assistive tech and does not accidentally submit a parent form.♿ Proposed fix
<button + type="button" class="inline-block p-3 btn-mini-primary w-12 focus:relative" data-testid="add-button" title={safeTranslate('add-' + data.model.localName)} + aria-label={safeTranslate('add-' + data.model.localName)} onclick={modalCreateForm} ><i class="fa-solid fa-file-circle-plus"></i> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte around lines 112 - 118, The icon-only add button lacks explicit semantics and can accidentally submit a parent form; update the button element used by modalCreateForm to include type="button" and an accessible name by adding an aria-label that mirrors the existing title (use safeTranslate('add-' + data.model.localName) for consistency), keeping the title prop as-is so assistive tech and sighted users both get the same label.backend/data_wizard/views.py (1)
5000-5003:⚠️ Potential issue | 🔴 CriticalUse
entity_namein Stakeholder creation (staleentitykwarg still present).Line 5002 and Line 5816 still write
entity=.... This is inconsistent with the migrated lookup field and can break stakeholder creation in both import paths.🔧 Proposed fix
@@ - stakeholder = Stakeholder.objects.create( + stakeholder = Stakeholder.objects.create( ebios_rm_study=study, - entity=entity_name, + entity_name=entity_name, third_party_entity=entity, category=category, @@ - stakeholder = Stakeholder.objects.create( + stakeholder = Stakeholder.objects.create( ebios_rm_study=study, - entity=entity_name, + entity_name=entity_name, third_party_entity=entity, category=category,Also applies to: 5814-5817
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/data_wizard/views.py` around lines 5000 - 5003, The Stakeholder creation calls use the stale kwarg entity=... instead of the migrated lookup field name entity_name, which can break creation; update all Stakeholder.objects.create(...) invocations (e.g., the calls that set ebios_rm_study and third_party_entity) to pass entity_name=entity_name (or the variable holding the entity string) rather than entity=..., ensuring both import paths (the occurrences around the shown diff and the duplicate at the other location) are changed consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/ebios_rm/models.py`:
- Around line 660-661: Update the model __str__ (method __str__ in the
stakeholder-like model where entity_name and third_party_entity exist) to avoid
rendering a leading space when entity_name is empty by choosing a fallback name:
compute name = self.entity_name.strip() if truthy else
(self.third_party_entity.name if self.third_party_entity else 'N/A'), then
return the string using that name with category.get_name_translated (preserving
the existing category logic). Apply the same fallback logic in the radar-chart
label construction in backend/ebios_rm/helpers.py (replace
f"{sh.entity_name}-{category_name}" with the computed fallback name + "-" +
category_name) so labels and logs use the linked third_party_entity name when
entity_name is empty.
- Line 650: The duplicate-detection signature omits the displayed human
identifier: update the fields_to_check list used for uniqueness checks
(currently defined as fields_to_check = ["ebios_rm_study", "third_party_entity",
"category"]) to include "entity_name" so that duplicate detection considers
entity_name alongside ebios_rm_study, third_party_entity and category.
- Around line 576-585: The model allows entity_name="" via default but lacks
blank=True and also permits both entity_name empty and third_party_entity null,
so update the field declaration for entity_name to include blank=True and add a
database-level constraint in the model Meta (or enforce in clean()) to require
at least one of entity_name or third_party_entity; implement this by adding a
CheckConstraint using Q (e.g., ~Q(entity_name="") |
Q(third_party_entity__isnull=False)) with a descriptive name like
"stakeholder_has_entity_identity" so the ORM/admin and DB consistently enforce
at least one identifier.
In `@backend/ebios_rm/tests/test_stakeholder.py`:
- Around line 21-31: The test currently only verifies the linked case where
entity_name is copied from third_party_entity; update the Stakeholder creation
in this test to use a distinct entity_name (not entity.name) and assert that
stakeholder.entity_name equals that distinct value to prove the free-text field
is stored independently, then add a separate test case creating a Stakeholder
with third_party_entity=None and a free-text entity_name, asserting the created
object has third_party_entity is None, entity_name equals the free-text value,
and that it appears in study.stakeholders.all(); target the Stakeholder model
and the test functions in backend/ebios_rm/tests/test_stakeholder.py when making
these changes.
In `@backend/ebios_rm/views.py`:
- Around line 503-506: The export loop dereferences sh.third_party_entity when
sh.entity_name is empty, causing an N+1 query; fix it by preloading the related
third_party_entity on the queryset used for the XLSX export (use
select_related('third_party_entity') or prefetch_related where appropriate) so
that the export iteration over sh (the rows used in the export function in
views.py) does not hit the DB per-row; update the queryset construction used
before the export loop (the variable producing sh items) to include the
relation.
In `@frontend/messages/ar.json`:
- Line 747: The Arabic translation for the key entity_name currently duplicates
the generic entity label and should be made distinct; update the value for
entity_name in frontend/messages/ar.json to a more specific Arabic label (for
example "اسم الكيان" or "اسم العنصر") that clearly differs from the existing
entity translation so both fields are unambiguous in the UI, and verify the new
string reads naturally in context where the UI shows both entity and
entity_name.
In `@frontend/messages/el.json`:
- Line 795: The Greek translation for the key entity_name is reusing the same
string as entity and should be a distinct, name-specific label; update the value
of entity_name to a clearer label like "Όνομα οντότητας" (or another concise
Greek equivalent) so it is distinguishable from the existing entity label in the
translations JSON.
In `@frontend/messages/es.json`:
- Line 720: The label key "entity_name" currently maps to the generic "Entidad"
which duplicates "entity"; update the Spanish translation for the "entity_name"
key to a clear name-field label such as "Nombre de la entidad" (or "Nombre de la
organización" if that fits your domain) so it explicitly indicates the
stakeholder's own entity name; locate the "entity_name" entry in the
translations file and replace its value with the chosen phrase, keeping JSON
syntax intact.
In `@frontend/messages/hr.json`:
- Around line 806-815: The translation key "entity_name" currently duplicates
"entity" and should be changed to a distinct label so the free-text entity name
is clearly different from the linked entity field; update the value for
"entity_name" (not "entity") in frontend/messages/hr.json to a clearer Croatian
phrase such as "Naziv entiteta" or "Naziv entiteta (slobodan tekst)" so the
form/export UI distinguishes the stakeholder’s free-text entity name from the
linked entity field.
In `@frontend/messages/id.json`:
- Line 751: The translation for the key "entity_name" currently equals "Entitas"
and collides with the existing "entity" label; update the "entity_name" value to
a distinct Indonesian string (e.g., "Nama entitas" or "Nama entitas (kustom)")
so users can differentiate the free-text field from the third-party "entity"
selector; locate the "entity_name" entry in the locale JSON and replace its
value accordingly.
In `@frontend/messages/it.json`:
- Line 729: The Italian translation for the key "entity_name" is ambiguous
because it duplicates the "entity" label; update the value for "entity_name" in
frontend/messages/it.json (key "entity_name") to a name-specific label such as
"Nome entità" (matching capitalization/style of the other keys) so the UI
clearly distinguishes it from the "entity" label.
In `@frontend/messages/ro.json`:
- Line 748: The current translation for "entity_name" duplicates "entity"
("Entitate"), obscuring the intended distinction; update the "entity_name"
translation to a clearer Romanian phrase such as "Nume entitate" or "Denumire
entitate" so it conveys a free-text entity/stakeholder name distinct from the
object/relationship label ("entity"). Locate the "entity_name" key in the
messages/ro.json resource and replace its value accordingly, keeping the
existing key unchanged.
In `@frontend/messages/tr.json`:
- Around line 804-805: The translation for third-party entity keys
thirdPartyEntity and thirdPartyEntityHelpText uses "varlığı" which can be
ambiguous; update both values to use the established terminology "Kuruluş"
(e.g., thirdPartyEntity: "Üçüncü taraf Kuruluş" and thirdPartyEntityHelpText:
"Paydaşı ilişkilendirmek istediğiniz üçüncü taraf Kuruluş.") so labels match
other entity/entity_name translations and avoid UI confusion.
In `@frontend/src/lib/components/Forms/ModelForm/StakeholderForm.svelte`:
- Around line 95-100: The reactive block watching selectedEntityOption currently
skips updating $formData.entity_name when context === 'edit', which allows
entity_name to become stale if third_party_entity is retargeted; change the
logic in the $effect that reads selectedEntityOption so that when a new entity
is selected you always set $formData.entity_name (or at least when the selected
entity's id/str differs from $formData.third_party_entity) rather than only when
context !== 'edit'; refer to the existing symbols selectedEntityOption, context,
$formData.entity_name and $formData.third_party_entity (or entity.id/entity.str)
and update the conditional to assign the entity.name/str into
$formData.entity_name in edit mode as well.
In `@frontend/src/lib/utils/schemas.ts`:
- Around line 1188-1189: The schema removed the old entity property but
consumers still reference stakeholder.entity; restore backward compatibility by
adding a computed alias property (e.g., keep or reintroduce an entity
getter/object) that maps the new entity_name and third_party_entity into the
original shape so existing code like stakeholder.entity.name,
stakeholder.entity.str, and conditional checks continue to work; update the
schema providers/util where entity_name and third_party_entity are defined
(symbols: entity_name, third_party_entity) to populate this alias, or
alternatively update the consumers in AttackPathFlowText.svelte,
AttackPathGraph.svelte, and report/+page.svelte to use
entity_name/third_party_entity everywhere before removing stakeholder.entity.
In `@frontend/src/lib/utils/table.ts`:
- Around line 2241-2261: The stakeholders filters map is incorrect: entity_name
is a plain string in StakeholderSchema but is mapped to ENTITY_FILTER (an entity
UUID picker); change the filter mapping so entity_name uses a text/string filter
(e.g., the same kind used for other string fields) or instead map the relation
field third_party_entity to ENTITY_FILTER if you intended to filter by related
entity UUIDs; update the stakeholders.filters object to replace ENTITY_FILTER
for entity_name with the appropriate text filter constant or swap to
third_party_entity -> ENTITY_FILTER and ensure stakeholders.head/body include
the matching column name.
---
Outside diff comments:
In `@backend/ebios_rm/serializers.py`:
- Around line 259-272: The entity_name field is marked read_only which prevents
imports from setting the stakeholder's standalone identifier; change the field
declaration in the Stakeholder serializer from entity_name =
serializers.CharField(read_only=True) to a writable CharField (e.g. entity_name
= serializers.CharField(required=True, allow_blank=False) or at minimum
entity_name = serializers.CharField()) so incoming payloads can populate the
identifier during import and recreate stakeholders without third_party_entity.
---
Duplicate comments:
In `@backend/data_wizard/views.py`:
- Around line 5000-5003: The Stakeholder creation calls use the stale kwarg
entity=... instead of the migrated lookup field name entity_name, which can
break creation; update all Stakeholder.objects.create(...) invocations (e.g.,
the calls that set ebios_rm_study and third_party_entity) to pass
entity_name=entity_name (or the variable holding the entity string) rather than
entity=..., ensuring both import paths (the occurrences around the shown diff
and the duplicate at the other location) are changed consistently.
In `@frontend/messages/de.json`:
- Line 720: The "entity_name" translation duplicates "entity" ("Entität") and is
ambiguous; update the "entity_name" key in frontend/messages/de.json to a
clearer German label (e.g., "Entitätsname" or "Name der Entität") to distinguish
the free-text name field from the entity selector, and verify the UI labels that
reference "entity" and "entity_name" (keys "entity" and "entity_name") remain
distinct and consistent across the app.
In `@frontend/messages/fr.json`:
- Line 1050: The translation key "entity_name" has an ambiguous French label
"Entité" that conflicts with the existing "entity" label; update the value for
the "entity_name" key to a clearer phrase such as "Nom de l'entité" so the
free-text field is distinguished from linked/entity reference labels (modify the
"entity_name" entry in frontend/messages/fr.json accordingly).
In `@frontend/messages/it.json`:
- Around line 737-738: The Italian translation for the key thirdPartyEntity is
currently plural; update the value for "thirdPartyEntity" from "Entità di terze
parti" to the singular wording "Entità di terza parte" and also make the help
text consistent by changing "thirdPartyEntityHelpText" from "L'entità di terze
parti a cui si desidera collegare la parte interessata." to "L'entità di terza
parte a cui si desidera collegare la parte interessata." to keep both labels
singular and aligned with the key.
In `@frontend/messages/nl.json`:
- Around line 728-729: Update the Dutch label for the "thirdPartyEntity" key to
be explicit that it refers to an entity/organization (e.g., change "Derde
partij" to "Entiteit/organisatie" or similar), and ensure the related
"thirdPartyEntityHelpText" still clarifies that this is the external entity you
want to link the stakeholder to; modify the values for the "thirdPartyEntity"
and, if needed for clarity, "thirdPartyEntityHelpText" entries in
frontend/messages/nl.json accordingly.
In
`@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte:
- Around line 112-118: The icon-only add button lacks explicit semantics and can
accidentally submit a parent form; update the button element used by
modalCreateForm to include type="button" and an accessible name by adding an
aria-label that mirrors the existing title (use safeTranslate('add-' +
data.model.localName) for consistency), keeping the title prop as-is so
assistive tech and sighted users both get the same label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81353229-1519-4900-951e-1b32e7a8e51a
📒 Files selected for processing (37)
backend/data_wizard/views.pybackend/ebios_rm/helpers.pybackend/ebios_rm/migrations/0025_remove_stakeholder_entity_stakeholder_entity_name_and_more.pybackend/ebios_rm/models.pybackend/ebios_rm/serializers.pybackend/ebios_rm/tests/test_stakeholder.pybackend/ebios_rm/views.pyfrontend/messages/ar.jsonfrontend/messages/cs.jsonfrontend/messages/da.jsonfrontend/messages/de.jsonfrontend/messages/el.jsonfrontend/messages/en.jsonfrontend/messages/es.jsonfrontend/messages/fr.jsonfrontend/messages/hi.jsonfrontend/messages/hr.jsonfrontend/messages/hu.jsonfrontend/messages/id.jsonfrontend/messages/it.jsonfrontend/messages/ko.jsonfrontend/messages/lt.jsonfrontend/messages/nl.jsonfrontend/messages/pl.jsonfrontend/messages/pt.jsonfrontend/messages/ro.jsonfrontend/messages/sv.jsonfrontend/messages/tr.jsonfrontend/messages/uk.jsonfrontend/messages/ur.jsonfrontend/messages/zh.jsonfrontend/src/lib/components/Forms/ModelForm/StakeholderForm.sveltefrontend/src/lib/utils/schemas.tsfrontend/src/lib/utils/table.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.sveltefrontend/tests/functional/detailed/ebios-rm.test.ts
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.ts (1)
47-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle fetch failures consistently.
When a fetch fails, the function returns
nullfrom the map callback but never populatesselectOptions[selectField.field]. Downstream code may expect allselectFieldsto have corresponding entries inselectOptions, causing potentialundefinedaccess errors.🛡️ Proposed fix to set an empty array on failure
const response = await fetch(url); if (!response.ok) { console.error(`Failed to fetch data from ${url}: ${response.statusText}`); - return null; + selectOptions[selectField.field] = []; + 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 `@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.ts around lines 47 - 50, The map callback that fetches options leaves selectOptions[selectField.field] unset when response.ok is false, returning null and causing downstream undefined access; modify the fetch failure branch inside the selectFields.map handler to assign an empty array to selectOptions[selectField.field] (and optionally log the error) and then continue (returning an appropriate empty value) so every selectField always has a defined entry in selectOptions; update the code paths around response.ok, the map callback, and any early returns so selectOptions is populated for each selectField even on fetch failure.backend/ebios_rm/views.py (1)
932-940:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpose
third_party_entityinStakeholderFilterfields.After renaming
entitytothird_party_entity, the filter list should includethird_party_entity; otherwise clients cannot filter by the linked entity relation anymore.💡 Proposed fix
fields = [ "ebios_rm_study", "is_selected", "applied_controls", "category", "entity_name", + "third_party_entity", ]🤖 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 `@backend/ebios_rm/views.py` around lines 932 - 940, The StakeholderFilter's Meta.fields list still references the old relation name and must include the renamed field so filtering works; update the Meta of the StakeholderFilter (the class with "class Meta" and model = Stakeholder) to include "third_party_entity" in the fields array (replace or add it alongside "entity_name"/other fields) so clients can filter by the linked entity relation again.
♻️ Duplicate comments (8)
frontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte (1)
112-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd accessible name and prevent accidental form submission.
The icon-only button lacks an
aria-label(thetitleattribute is not reliably announced by screen readers) and should specifytype="button"to prevent accidental form submission if rendered within a form context.♿ Proposed fix
<button + type="button" class="inline-block p-3 btn-mini-primary w-12 focus:relative" data-testid="add-button" title={safeTranslate('add-' + data.model.localName)} + aria-label={safeTranslate('add-' + data.model.localName)} onclick={modalCreateForm} ><i class="fa-solid fa-file-circle-plus"></i> </button>🤖 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 `@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte around lines 112 - 118, The icon-only add button (onclick={modalCreateForm}) needs an accessible name and must not submit forms: add type="button" to the button element and add an aria-label using the same translated label as the title (e.g. aria-label={safeTranslate('add-' + data.model.localName)}) so screen readers get a proper name; keep the existing title if desired but ensure aria-label is present and type is "button".frontend/messages/de.json (1)
721-721:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a more specific label for
entity_nameto avoid field ambiguity.The translation
"Entität"duplicates the genericentitylabel at line 720, which can confuse users when both fields are visible. Use a more specific German label such as"Entitätsname"or"Name der Entität"to distinguish the fields.🤖 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 `@frontend/messages/de.json` at line 721, The label for the i18n key "entity_name" is too generic and duplicates the "entity" label; update the value for the key "entity_name" in the frontend/messages/de.json translations from "Entität" to a more specific German label such as "Entitätsname" or "Name der Entität" so the two fields ("entity" and "entity_name") are clearly distinguishable in the UI.frontend/messages/el.json (1)
798-798:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe
entity_namelabel remains indistinguishable fromentity.This issue was previously flagged: both keys use "Οντότητα", making them identical in the UI. Update
entity_nameto a distinct label such as "Όνομα οντότητας" to differentiate it from the existing entity label.✏️ Suggested fix
- "entity_name": "Οντότητα", + "entity_name": "Όνομα οντότητας",🤖 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 `@frontend/messages/el.json` at line 798, The translation key "entity_name" currently uses the same Greek label as "entity" ("Οντότητα"), causing UI ambiguity; update the value of the "entity_name" key to a distinct label such as "Όνομα οντότητας" so it is clearly distinguishable from "entity" (look for the "entity_name" JSON key in the frontend/messages/el.json and replace its value).frontend/messages/es.json (1)
724-724:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake
entity_nameread as a name field.This issue was previously identified but remains unresolved.
entity_namecurrently uses"Entidad", which duplicates the existingentitylabel (line 723). Please use a label that clearly denotes the stakeholder's own entity name, such as"Nombre de la entidad".Suggested fix
- "entity_name": "Entidad", + "entity_name": "Nombre de la entidad",🤖 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 `@frontend/messages/es.json` at line 724, The Spanish translation key "entity_name" currently has the value "Entidad" which duplicates the nearby "entity" label; update the translation value for the "entity_name" key to a clear name-field label such as "Nombre de la entidad" so it denotes the stakeholder's own entity name (locate the "entity_name" JSON key in frontend/messages/es.json alongside the "entity" key and replace its string value).backend/ebios_rm/models.py (2)
650-650:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
entity_namein duplicate detection keys.With the new schema, omitting
entity_namefromfields_to_checkcan incorrectly treat distinct free-text stakeholders as duplicates whenthird_party_entityis null.💡 Proposed fix
- fields_to_check = ["ebios_rm_study", "third_party_entity", "category"] + fields_to_check = ["ebios_rm_study", "entity_name", "third_party_entity", "category"]🤖 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 `@backend/ebios_rm/models.py` at line 650, The duplicate-detection keys in models.py omit the free-text stakeholder column, so distinct free-text entities can be treated as duplicates; update the fields_to_check list (the variable named fields_to_check used for duplicate detection) to include "entity_name" alongside "ebios_rm_study", "third_party_entity", and "category" so that comparisons consider the free-text stakeholder value when third_party_entity is null.
576-585:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce stakeholder identity at persistence level.
entity_name=""andthird_party_entity=NULLcan still be stored, which creates unnamed stakeholders and weakens data integrity. Require at least one identifier.💡 Proposed fix
- entity_name = models.TextField(verbose_name=_("Entity name"), default="") + entity_name = models.TextField( + verbose_name=_("Entity name"), + default="", + blank=True, + ) class Meta: verbose_name = _("Stakeholder") verbose_name_plural = _("Stakeholders") ordering = ["created_at"] + constraints = [ + models.CheckConstraint( + check=~Q(entity_name="") | Q(third_party_entity__isnull=False), + name="stakeholder_has_entity_identity", + ), + ]🤖 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 `@backend/ebios_rm/models.py` around lines 576 - 585, Add a persistence-level requirement that either entity_name is non-empty or third_party_entity is set: implement a clean() method on the model that raises django.core.exceptions.ValidationError if both self.entity_name == "" and self.third_party_entity is None (and call full_clean() from save() or ensure callers run full_clean()), and add a Django CheckConstraint in the model Meta using condition=(~Q(entity_name="") | Q(third_party_entity__isnull=False)) with a descriptive name (e.g., "stakeholder_require_name_or_entity") so the DB also enforces the rule; reference the existing fields entity_name and third_party_entity when making these changes.backend/ebios_rm/helpers.py (1)
139-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the same fallback logic for chart labels as export/UI.
f"{sh.entity_name}-{category_name}"produces blank labels whenentity_nameis empty. Fall back tothird_party_entity.namebefore building labels.💡 Proposed fix
+ display_name = sh.entity_name or ( + sh.third_party_entity.name if sh.third_party_entity else "" + ) current_data[c_maturity_group].append( [ c_criticality, angle, c_exposure_val, - f"{sh.entity_name}-{category_name}", + f"{display_name}-{category_name}", ] ) @@ residual_data[r_maturity_group].append( [ r_criticality, angle, r_exposure_val, - f"{sh.entity_name}-{category_name}", + f"{display_name}-{category_name}", ] )Also applies to: 158-164
🤖 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 `@backend/ebios_rm/helpers.py` around lines 139 - 145, The chart label uses f"{sh.entity_name}-{category_name}" which can be blank when sh.entity_name is empty; update the label construction in the current_data append (the block that builds [c_criticality, angle, c_exposure_val, ...]) to use the same fallback logic as the export/UI: use sh.entity_name if non-empty otherwise use third_party_entity.name before concatenating with category_name; apply the same change to the second occurrence around the other current_data append (the block covering the 158-164 equivalent) so both label places consistently fall back to third_party_entity.name.backend/ebios_rm/views.py (1)
503-506:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid N+1 queries in stakeholder XLSX export fallback.
The fallback now reads
sh.third_party_entity.nameinside the loop; preloadthird_party_entityon the queryset used for export.💡 Proposed fix
- stakeholders = Stakeholder.objects.filter(ebios_rm_study=study) + stakeholders = ( + Stakeholder.objects.filter(ebios_rm_study=study) + .select_related("third_party_entity") + )🤖 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 `@backend/ebios_rm/views.py` around lines 503 - 506, The export loop accesses sh.third_party_entity.name causing N+1 queries; update the queryset used for the stakeholder XLSX export (the QuerySet variable built in the export/fallback function in backend/ebios_rm/views.py) to eager-load the relation by adding select_related('third_party_entity') (or the appropriate related field name) so sh.third_party_entity is already fetched when iterating; ensure you modify the QuerySet construction rather than the loop itself and keep other related preloads intact.
🤖 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.
Inline comments:
In `@frontend/messages/cs.json`:
- Line 751: The JSON key "entity_name" currently uses the same Czech label as
"entity" ("Entita"), making them indistinguishable; update the value for
"entity_name" to a more specific Czech label (for example "Název entity" or
"Jméno entity") so forms and tables clearly differentiate it from "entity" —
change the string assigned to the "entity_name" key accordingly.
In `@frontend/messages/da.json`:
- Line 763: The Danish translation for the "entity_name" key is identical to
"entity" ("Enhed"), which can confuse users; update the "entity_name" value in
frontend/messages/da.json (key "entity_name") to a distinct Danish label such as
"Enhedsnavn" or "Navn på enhed" so it clearly differs from the "entity"
translation.
In `@frontend/messages/en.json`:
- Line 1185: The label for the i18n key "entity_name" is ambiguous; update its
value to a more explicit string such as "Entity name" so it clearly denotes a
free-text input vs linked entity labels (refer to the "entity_name" key in
frontend/messages/en.json). Replace the current value "Entity" with "Entity
name" and ensure any UI components using the "entity_name" key still render
correctly after the change.
In `@frontend/messages/hi.json`:
- Line 750: The Hindi translation key "entity_name" currently duplicates the
"entity" label ("इकाई") which makes free-text entity name indistinguishable from
the linked entity field; update the "entity_name" value in
frontend/messages/hi.json to a clearer Hindi phrase (e.g., "इकाई का नाम" or
similar) that explicitly indicates a free-text name, ensuring it remains concise
and consistent with UX conventions and other keys like "entity".
In `@frontend/messages/hu.json`:
- Line 753: The Hungarian translation key "entity_name" currently duplicates the
label for "entity" and must be disambiguated so the free-text stakeholder/name
field is clearly different from the linked entity field; update the value of
"entity_name" in the hu.json translations to a distinct Hungarian label (for
example "Entitás neve (szabad szöveg)" or "Kapcsolt entitás neve") so users can
tell it is the editable name field while leaving the "entity" label as the
linked/entity selector.
In `@frontend/messages/nl.json`:
- Line 721: The Dutch translation for the JSON key "entity_name" duplicates
"entity" ("Entiteit"), causing UI ambiguity; update the value for the
"entity_name" key in frontend/messages/nl.json to a name-specific label such as
"Entiteitsnaam" so it is clearly distinguishable from the "entity" label.
In `@frontend/messages/pl.json`:
- Line 799: Update the Polish translation for the key "entity_name" so it is
distinct from the "entity" label; replace the current value "Podmiot" with a
disambiguating phrase such as "Nazwa podmiotu" (or another appropriate Polish
wording) to clearly indicate a free-text stakeholder name rather than the linked
entity selection, ensuring only the value for the "entity_name" key is changed
and "entity" remains "Podmiot".
In `@frontend/messages/pt.json`:
- Line 751: The "entity_name" translation currently duplicates the "entity"
label and should be made distinct; update the value of the "entity_name" key in
frontend/messages/pt.json to a clearer Portuguese label (for example "Nome da
entidade" or another context-appropriate phrase) so it is easily distinguishable
from the existing "entity" string; locate the "entity_name" key in the JSON and
replace its value accordingly.
In `@frontend/messages/zh.json`:
- Line 944: Replace the ambiguous Chinese label for the key "entity_name"
(currently "实体") with a more specific form-label such as "实体名称" so it clearly
distinguishes the name field from the general "entity" label; update the value
of the JSON key "entity_name" to "实体名称" in frontend/messages/zh.json (leave
other keys like "entity" unchanged).
---
Outside diff comments:
In `@backend/ebios_rm/views.py`:
- Around line 932-940: The StakeholderFilter's Meta.fields list still references
the old relation name and must include the renamed field so filtering works;
update the Meta of the StakeholderFilter (the class with "class Meta" and model
= Stakeholder) to include "third_party_entity" in the fields array (replace or
add it alongside "entity_name"/other fields) so clients can filter by the linked
entity relation again.
In
`@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.ts:
- Around line 47-50: The map callback that fetches options leaves
selectOptions[selectField.field] unset when response.ok is false, returning null
and causing downstream undefined access; modify the fetch failure branch inside
the selectFields.map handler to assign an empty array to
selectOptions[selectField.field] (and optionally log the error) and then
continue (returning an appropriate empty value) so every selectField always has
a defined entry in selectOptions; update the code paths around response.ok, the
map callback, and any early returns so selectOptions is populated for each
selectField even on fetch failure.
---
Duplicate comments:
In `@backend/ebios_rm/helpers.py`:
- Around line 139-145: The chart label uses f"{sh.entity_name}-{category_name}"
which can be blank when sh.entity_name is empty; update the label construction
in the current_data append (the block that builds [c_criticality, angle,
c_exposure_val, ...]) to use the same fallback logic as the export/UI: use
sh.entity_name if non-empty otherwise use third_party_entity.name before
concatenating with category_name; apply the same change to the second occurrence
around the other current_data append (the block covering the 158-164 equivalent)
so both label places consistently fall back to third_party_entity.name.
In `@backend/ebios_rm/models.py`:
- Line 650: The duplicate-detection keys in models.py omit the free-text
stakeholder column, so distinct free-text entities can be treated as duplicates;
update the fields_to_check list (the variable named fields_to_check used for
duplicate detection) to include "entity_name" alongside "ebios_rm_study",
"third_party_entity", and "category" so that comparisons consider the free-text
stakeholder value when third_party_entity is null.
- Around line 576-585: Add a persistence-level requirement that either
entity_name is non-empty or third_party_entity is set: implement a clean()
method on the model that raises django.core.exceptions.ValidationError if both
self.entity_name == "" and self.third_party_entity is None (and call
full_clean() from save() or ensure callers run full_clean()), and add a Django
CheckConstraint in the model Meta using condition=(~Q(entity_name="") |
Q(third_party_entity__isnull=False)) with a descriptive name (e.g.,
"stakeholder_require_name_or_entity") so the DB also enforces the rule;
reference the existing fields entity_name and third_party_entity when making
these changes.
In `@backend/ebios_rm/views.py`:
- Around line 503-506: The export loop accesses sh.third_party_entity.name
causing N+1 queries; update the queryset used for the stakeholder XLSX export
(the QuerySet variable built in the export/fallback function in
backend/ebios_rm/views.py) to eager-load the relation by adding
select_related('third_party_entity') (or the appropriate related field name) so
sh.third_party_entity is already fetched when iterating; ensure you modify the
QuerySet construction rather than the loop itself and keep other related
preloads intact.
In `@frontend/messages/de.json`:
- Line 721: The label for the i18n key "entity_name" is too generic and
duplicates the "entity" label; update the value for the key "entity_name" in the
frontend/messages/de.json translations from "Entität" to a more specific German
label such as "Entitätsname" or "Name der Entität" so the two fields ("entity"
and "entity_name") are clearly distinguishable in the UI.
In `@frontend/messages/el.json`:
- Line 798: The translation key "entity_name" currently uses the same Greek
label as "entity" ("Οντότητα"), causing UI ambiguity; update the value of the
"entity_name" key to a distinct label such as "Όνομα οντότητας" so it is clearly
distinguishable from "entity" (look for the "entity_name" JSON key in the
frontend/messages/el.json and replace its value).
In `@frontend/messages/es.json`:
- Line 724: The Spanish translation key "entity_name" currently has the value
"Entidad" which duplicates the nearby "entity" label; update the translation
value for the "entity_name" key to a clear name-field label such as "Nombre de
la entidad" so it denotes the stakeholder's own entity name (locate the
"entity_name" JSON key in frontend/messages/es.json alongside the "entity" key
and replace its string value).
In
`@frontend/src/routes/`(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.svelte:
- Around line 112-118: The icon-only add button (onclick={modalCreateForm})
needs an accessible name and must not submit forms: add type="button" to the
button element and add an aria-label using the same translated label as the
title (e.g. aria-label={safeTranslate('add-' + data.model.localName)}) so screen
readers get a proper name; keep the existing title if desired but ensure
aria-label is present and type is "button".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f99acd32-504f-42be-bfde-8d62f7bc31ab
📒 Files selected for processing (37)
backend/data_wizard/views.pybackend/ebios_rm/helpers.pybackend/ebios_rm/migrations/0025_remove_stakeholder_entity_stakeholder_entity_name_and_more.pybackend/ebios_rm/models.pybackend/ebios_rm/serializers.pybackend/ebios_rm/tests/test_stakeholder.pybackend/ebios_rm/views.pyfrontend/messages/ar.jsonfrontend/messages/cs.jsonfrontend/messages/da.jsonfrontend/messages/de.jsonfrontend/messages/el.jsonfrontend/messages/en.jsonfrontend/messages/es.jsonfrontend/messages/fr.jsonfrontend/messages/hi.jsonfrontend/messages/hr.jsonfrontend/messages/hu.jsonfrontend/messages/id.jsonfrontend/messages/it.jsonfrontend/messages/ko.jsonfrontend/messages/lt.jsonfrontend/messages/nl.jsonfrontend/messages/pl.jsonfrontend/messages/pt.jsonfrontend/messages/ro.jsonfrontend/messages/sv.jsonfrontend/messages/tr.jsonfrontend/messages/uk.jsonfrontend/messages/ur.jsonfrontend/messages/zh.jsonfrontend/src/lib/components/Forms/ModelForm/StakeholderForm.sveltefrontend/src/lib/utils/schemas.tsfrontend/src/lib/utils/table.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.sveltefrontend/tests/functional/detailed/ebios-rm.test.ts
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/data_wizard/views.py (1)
5042-5146:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBackfill
third_party_entityin UPDATE mode too.These paths now set
third_party_entityonly on create. WhenConflictMode.UPDATEmatches an existing stakeholder byentity_name, the resolvedEntitynever gets written back, so pre-migration rows or earlier imports stay unlinked after re-import.🔧 Suggested fix
case ConflictMode.UPDATE: + existing_stakeholder.third_party_entity = entity existing_stakeholder.is_selected = ( stakeholder_data.get( "is_selected", @@ case ConflictMode.UPDATE: + existing_sh.third_party_entity = entity val = sh_data.get("current_dependency") if val is not None: existing_sh.current_dependency = val @@ case ConflictMode.UPDATE: + existing.third_party_entity = entity existing.current_dependency = dep existing.current_penetration = pen existing.current_maturity = matAlso applies to: 5875-5949, 6664-6707
🤖 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 `@backend/data_wizard/views.py` around lines 5042 - 5146, When handling ConflictMode.UPDATE for an existing Stakeholder, the resolved Entity (variable entity) is not assigned back to existing_stakeholder.third_party_entity, so imports don’t backfill that relation; update the ConflictMode.UPDATE branch to set existing_stakeholder.third_party_entity = entity (and any other missing fields) before calling existing_stakeholder.save() in the block inside the stakeholder upsert logic (the existing_stakeholder, third_party_entity, ConflictMode.UPDATE, stakeholder_data, and save() symbols identify the spot); apply the same change to the other analogous UPDATE blocks noted in the review.
♻️ Duplicate comments (4)
frontend/messages/de.json (1)
721-721:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse "Entität" instead of "Einheit" for consistency and clarity.
At Line 721, the translation "Name der Einheit" uses "Einheit" which breaks consistency with the established entity terminology in this file (e.g., lines 720, 722, 723 all use "Entität"). Additionally, the past review comment correctly identified that a more specific label is needed. Use "Entitätsname" or "Name der Entität" to:
- Maintain consistency with existing entity-related translations
- Distinguish this field from the generic "entity" label (line 720)
- Align with standard German technical terminology where "Entität" is preferred for database/domain entities
🔄 Proposed fix
- "entity_name": "Name der Einheit", + "entity_name": "Entitätsname",or
- "entity_name": "Name der Einheit", + "entity_name": "Name der Entität",🤖 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 `@frontend/messages/de.json` at line 721, Update the translation value for the "entity_name" key to use "Entität" terminology for consistency; replace the current "Name der Einheit" with either "Entitätsname" or "Name der Entität" so it matches other entity-related translations in this file and aligns with standard German technical wording.frontend/messages/it.json (2)
741-742:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevious feedback not addressed: Use singular "terza parte" instead of plural.
This issue was already raised in an earlier review. Since the field refers to a single third-party entity (not multiple), the singular form is more appropriate.
✏️ Suggested fix
- "thirdPartyEntity": "Entità di terze parti", - "thirdPartyEntityHelpText": "L'entità di terze parti a cui si desidera collegare la parte interessata.", + "thirdPartyEntity": "Entità di terza parte", + "thirdPartyEntityHelpText": "L'entità di terza parte a cui si desidera collegare la parte interessata.",🤖 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 `@frontend/messages/it.json` around lines 741 - 742, Change the Italian translation values for the keys "thirdPartyEntity" and "thirdPartyEntityHelpText" to use the singular phrase "terza parte" instead of the plural "terze parti"/"terze parti" so they correctly refer to a single third-party entity; update the value for "thirdPartyEntity" and adjust "thirdPartyEntityHelpText" to read something like "L'entità di terza parte a cui si desidera collegare la parte interessata." ensuring grammar and apostrophe usage are correct.
733-733:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevious feedback not addressed: Differentiate
entity_namefromentity.This was already flagged in an earlier review. Using "Nome dell'entità" doesn't clearly distinguish this field from the existing "entity" label. Consider the more concise "Nome entità" which better matches UI label patterns elsewhere (e.g., line 754: "Entità fornitore").
✏️ Suggested fix
- "entity_name": "Nome dell'entità", + "entity_name": "Nome entità",🤖 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 `@frontend/messages/it.json` at line 733, The Italian translation for the key "entity_name" currently reads "Nome dell'entità" which is too similar to the existing "entity" label; change the value of the "entity_name" key to "Nome entità" to match the concise UI label pattern (see other translation like "Entità fornitore") and keep labeling consistent across the locale file.backend/ebios_rm/tests/test_stakeholder.py (1)
21-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest coverage still insufficient for the optional relation.
The past review comment remains unaddressed. The test only verifies the linked case where
entity_namemirrors the linked entity's name, which doesn't prove the free-text field stores independently. Additionally, there's no test case forthird_party_entity=Noneto verify the core "optional" behavior introduced by this PR.Please:
- Use a distinct
entity_namevalue (notentity.name) in this test to prove independence- Add a separate test case with
third_party_entity=Noneand a free-textentity_name🤖 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 `@backend/ebios_rm/tests/test_stakeholder.py` around lines 21 - 31, Update the tests to prove the optional relation stores free-text independently: in the existing test that calls Stakeholder.objects.create(... third_party_entity=entity, entity_name=entity.name, ...), change entity_name to a distinct literal (e.g., "Custom Name") and assert stakeholder.entity_name == "Custom Name" and stakeholder.third_party_entity == entity; then add a new test that creates a Stakeholder with third_party_entity=None and entity_name set to a free-text value, save it, and assert the instance is present in study.stakeholders.all(), has third_party_entity is None, and entity_name matches the free-text string (use the same Stakeholder.objects.create pattern to locate the code to modify).
🧹 Nitpick comments (1)
frontend/messages/de.json (1)
729-729: ⚡ Quick winConsider a more explicit translation that includes "Entität".
At Line 729, "thirdPartyEntity" is translated as "Drittpartei" which means "third party" but omits the "entity" component. This could create ambiguity when users need to distinguish between:
- A general third-party relationship (line 727: "Dritte")
- A specific third-party entity reference (this key)
Consider "Drittpartei-Entität" or "Drittanbieter-Entität" to make it clear this refers to an entity record, not just a category or relationship type.
💡 Proposed alternatives
- "thirdPartyEntity": "Drittpartei", + "thirdPartyEntity": "Drittpartei-Entität",or
- "thirdPartyEntity": "Drittpartei", + "thirdPartyEntity": "Drittanbieter-Entität",🤖 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 `@frontend/messages/de.json` at line 729, Update the German translation value for the "thirdPartyEntity" key to explicitly include "Entität" so it unambiguously refers to an entity record (e.g., change value from "Drittpartei" to "Drittpartei-Entität" or "Drittanbieter-Entität"); locate the "thirdPartyEntity" entry in frontend/messages/de.json and replace the string value accordingly, keeping formatting and punctuation consistent with the surrounding JSON entries.
🤖 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.
Inline comments:
In `@frontend/messages/tr.json`:
- Line 799: Replace the inconsistent Turkish translation for the key
"entity_name" (currently "Varlık adı") with "Kuruluş adı" to match the existing
"entity" translation ("Kuruluş") and avoid confusing it with the asset
translation "Varlık"; update the translation string for the "entity_name" entry
so it consistently represents an organization name across the messages.
---
Outside diff comments:
In `@backend/data_wizard/views.py`:
- Around line 5042-5146: When handling ConflictMode.UPDATE for an existing
Stakeholder, the resolved Entity (variable entity) is not assigned back to
existing_stakeholder.third_party_entity, so imports don’t backfill that
relation; update the ConflictMode.UPDATE branch to set
existing_stakeholder.third_party_entity = entity (and any other missing fields)
before calling existing_stakeholder.save() in the block inside the stakeholder
upsert logic (the existing_stakeholder, third_party_entity, ConflictMode.UPDATE,
stakeholder_data, and save() symbols identify the spot); apply the same change
to the other analogous UPDATE blocks noted in the review.
---
Duplicate comments:
In `@backend/ebios_rm/tests/test_stakeholder.py`:
- Around line 21-31: Update the tests to prove the optional relation stores
free-text independently: in the existing test that calls
Stakeholder.objects.create(... third_party_entity=entity,
entity_name=entity.name, ...), change entity_name to a distinct literal (e.g.,
"Custom Name") and assert stakeholder.entity_name == "Custom Name" and
stakeholder.third_party_entity == entity; then add a new test that creates a
Stakeholder with third_party_entity=None and entity_name set to a free-text
value, save it, and assert the instance is present in study.stakeholders.all(),
has third_party_entity is None, and entity_name matches the free-text string
(use the same Stakeholder.objects.create pattern to locate the code to modify).
In `@frontend/messages/de.json`:
- Line 721: Update the translation value for the "entity_name" key to use
"Entität" terminology for consistency; replace the current "Name der Einheit"
with either "Entitätsname" or "Name der Entität" so it matches other
entity-related translations in this file and aligns with standard German
technical wording.
In `@frontend/messages/it.json`:
- Around line 741-742: Change the Italian translation values for the keys
"thirdPartyEntity" and "thirdPartyEntityHelpText" to use the singular phrase
"terza parte" instead of the plural "terze parti"/"terze parti" so they
correctly refer to a single third-party entity; update the value for
"thirdPartyEntity" and adjust "thirdPartyEntityHelpText" to read something like
"L'entità di terza parte a cui si desidera collegare la parte interessata."
ensuring grammar and apostrophe usage are correct.
- Line 733: The Italian translation for the key "entity_name" currently reads
"Nome dell'entità" which is too similar to the existing "entity" label; change
the value of the "entity_name" key to "Nome entità" to match the concise UI
label pattern (see other translation like "Entità fornitore") and keep labeling
consistent across the locale file.
---
Nitpick comments:
In `@frontend/messages/de.json`:
- Line 729: Update the German translation value for the "thirdPartyEntity" key
to explicitly include "Entität" so it unambiguously refers to an entity record
(e.g., change value from "Drittpartei" to "Drittpartei-Entität" or
"Drittanbieter-Entität"); locate the "thirdPartyEntity" entry in
frontend/messages/de.json and replace the string value accordingly, keeping
formatting and punctuation consistent with the surrounding JSON entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bcb61fc-009b-4173-b7f1-9334dcfa16b5
📒 Files selected for processing (38)
backend/data_wizard/views.pybackend/ebios_rm/helpers.pybackend/ebios_rm/migrations/0025_remove_stakeholder_entity_stakeholder_entity_name_and_more.pybackend/ebios_rm/models.pybackend/ebios_rm/serializers.pybackend/ebios_rm/tests/test_stakeholder.pybackend/ebios_rm/views.pyfrontend/messages/ar.jsonfrontend/messages/cs.jsonfrontend/messages/da.jsonfrontend/messages/de.jsonfrontend/messages/el.jsonfrontend/messages/en.jsonfrontend/messages/es.jsonfrontend/messages/et.jsonfrontend/messages/fr.jsonfrontend/messages/hi.jsonfrontend/messages/hr.jsonfrontend/messages/hu.jsonfrontend/messages/id.jsonfrontend/messages/it.jsonfrontend/messages/ko.jsonfrontend/messages/lt.jsonfrontend/messages/nl.jsonfrontend/messages/pl.jsonfrontend/messages/pt.jsonfrontend/messages/ro.jsonfrontend/messages/sv.jsonfrontend/messages/tr.jsonfrontend/messages/uk.jsonfrontend/messages/ur.jsonfrontend/messages/zh.jsonfrontend/src/lib/components/Forms/ModelForm/StakeholderForm.sveltefrontend/src/lib/utils/schemas.tsfrontend/src/lib/utils/table.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.server.tsfrontend/src/routes/(app)/(internal)/ebios-rm/[id=uuid]/workshop-3/ecosystem/+page.sveltefrontend/tests/functional/detailed/ebios-rm.test.ts
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
1 participant
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.
This PR unlink the ebiosRM's stakeholder and third party entities. Now, a stakeholder can relate to a third party entity but it is optional. The field is now called "third_party_entity".
In order to remediate to the possible lack of entity in the stakeholder, the PR adds an entity field that take the name of the stakeholder's entity, without it being a third-party entity.
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Tests
Documentation