N°8860 - Régression Export CSV clé externe, plus de critères#971
N°8860 - Régression Export CSV clé externe, plus de critères#971bdalsass wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes CSV export handling for namespaced external-key and stopwatch attributes.
Changes:
- Uses fully qualified attribute class constants in subtype detection.
- Restores external-key export criteria generation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| Filename | Overview |
|---|---|
| core/tabularbulkexport.class.inc.php | Replaces bare string class name literals with ::class constants (and adds matching use declarations) in the GetSubAttributes switch, fixing the namespace mismatch that prevented external-key columns from being recognized during CSV export. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as Export UI
participant TBE as TabularBulkExport
participant MM as MetaModel
UI->>TBE: GetInteractiveFieldsWidget()
TBE->>MM: ListAttributeDefs(sClass)
MM-->>TBE: oAttDef instances (namespaced FQCNs)
TBE->>TBE: GetSubAttributes(sClass, sAttCode, oAttDef)
Note over TBE: switch(get_class($oAttDef))<br/>Before: 'AttributeExternalKey' never matched FQCN<br/>After: AttributeExternalKey::class matches correctly
alt FQCN matches case
TBE->>MM: GetAttributeDef / GetTargetClass
MM-->>TBE: remote class info
TBE->>TBE: Build sub-attributes (id, friendlyname, ext fields)
end
TBE-->>UI: aAllFields with external key sub-attributes
UI->>UI: Render field selector with criteria columns
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as Export UI
participant TBE as TabularBulkExport
participant MM as MetaModel
UI->>TBE: GetInteractiveFieldsWidget()
TBE->>MM: ListAttributeDefs(sClass)
MM-->>TBE: oAttDef instances (namespaced FQCNs)
TBE->>TBE: GetSubAttributes(sClass, sAttCode, oAttDef)
Note over TBE: switch(get_class($oAttDef))<br/>Before: 'AttributeExternalKey' never matched FQCN<br/>After: AttributeExternalKey::class matches correctly
alt FQCN matches case
TBE->>MM: GetAttributeDef / GetTargetClass
MM-->>TBE: remote class info
TBE->>TBE: Build sub-attributes (id, friendlyname, ext fields)
end
TBE-->>UI: aAllFields with external key sub-attributes
UI->>UI: Render field selector with criteria columns
Comments Outside Diff (1)
-
core/tabularbulkexport.class.inc.php, line 93-98 (link)get_class()performs an exact class-name match, so any third-party subclass ofAttributeExternalKeythat isn'tAttributeHierarchicalKeywill silently fall through the switch and produce no sub-attributes. Aninstanceof-based approach handles the full class hierarchy without extra cases.
Reviews (1): Last reviewed commit: "N°8860 - Régression Export CSV clé exter..." | Re-trigger Greptile
| case 'AttributeStopWatch': | ||
| case AttributeStopWatch::class: | ||
| foreach (MetaModel::ListAttributeDefs($sClass) as $sSubAttCode => $oSubAttDef) { | ||
| if ($oSubAttDef instanceof AttributeSubItem) { |
|
|
||
| protected function IsSubAttribute($sClass, $sAttCode, $oAttDef) | ||
| { | ||
| return (($oAttDef instanceof AttributeFriendlyName) || ($oAttDef instanceof AttributeExternalField) || ($oAttDef instanceof AttributeSubItem)); |
There was a problem hiding this comment.
There might be a missing use for the AttributeFriendlyMame and AttributeExternalField classes.
It may not be mandatory for now as there are the class aliases, but having some use in the file header could lead to a misunderstanding and letting people think that all Attribute classes have be migrated.
| if (!is_null($sFriendlyNameAttCode)) { | ||
| // The friendly name is made of a single attribute, check if that attribute is present as an external field | ||
| foreach (MetaModel::ListAttributeDefs($sClass) as $sSubAttCode => $oSubAttDef) { | ||
| if ($oSubAttDef instanceof AttributeExternalField) { |
| } | ||
|
|
||
| foreach (MetaModel::ListAttributeDefs($sClass) as $sSubAttCode => $oSubAttDef) { | ||
| if ($oSubAttDef instanceof AttributeExternalField) { |
Base information
Symptom (bug) / Objective (enhancement)
Export CSV do not handle correctly attributes external key
Reproduction procedure (bug)
Export location, organization field is not recognized as external key attribute
Checklist before requesting a review