Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"BT-60-Lot" in notice subtype 16.json seems to be located under the incorrect nodeId #799

Open
VirtualSatai opened this issue Jan 10, 2024 · 3 comments
Labels
notice-types Related to the notice type definitions (/notice-types).

Comments

@VirtualSatai
Copy link

I'm trying to convert a physical model into a visual model as defined by the notice editor sample project, using the 'TEDEFO-2456-use-of-fields-attribute-information' branch as a base.

Problem:

But some of the fields are not getting translated correctly. I'm working with the file: https://github.com/OP-TED/eForms-SDK/blob/develop/examples/notices/cn_24_minimal.xml#L138 and BT-60-Lot: XML node: "cbc:FundingProgramCode" is not working. I'm using the relative XPath to navigate the physical model.

I'm finding that there might be an issue in the notice type definition of notice subtype 16.

In fields.json: "BT-60-Lot" has the parentNodeId: "ND-LotTenderingTerms" https://github.com/OP-TED/eForms-SDK/blob/develop/fields/fields.json#L26932. "ND-LotTenderingTerms" is a direct child of "ND-Lot": https://github.com/OP-TED/eForms-SDK/blob/develop/fields/fields.json#L408

In 16.json (and possibly others), "BT-60-Lot" the nearest parent "nodeId" is "ND-LotTenderingProcess", also a direct child of "ND-Lot": https://github.com/OP-TED/eForms-SDK/blob/develop/fields/fields.json#L289

Suspected solution

Since "BT-60-Lot" is placed under the wrong nodeId, I'm finding that the processing doesn't work. I'd expect "BT-60-Lot" to be placed under "ND-LotTenderingTerms" or perhaps "ND-Lot" instead.

@VirtualSatai
Copy link
Author

VirtualSatai commented Jan 11, 2024

Looking further into this issue I find that the nodeId: ND-LotTenderingProcess is misplaced in many of the notice types (7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, CEI, T01, T02). I've found that simply removing the "nodeId": "ND-LotTenderingProcess" line from the notice-type json file makes my code work...

For notice type 16 in SDK version 1.9.1 i find the following fields has a wrong nodeId on their parent:

| GR-Lot | ContentIds: [notice-data] | noticeNodeIds: [ND-Root]
    | GR-Lot-Purpose | ContentIds: [notice-data; GR-Lot] | noticeNodeIds: [ND-Root; ND-Lot]
        | GR-Lot-PreviousPlanning | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-Description | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-22-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | --- WRONG nodeId --- | BT-21-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | --- WRONG nodeId --- | BT-24-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | --- WRONG nodeId --- | BT-23-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | GR-Lot-Additional-Nature | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-Description] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-Scope | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-25-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | --- WRONG nodeId --- | BT-625-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | --- WRONG nodeId --- | BT-726-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'
            | --- WRONG nodeId --- | BT-27-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotValueEstimate'
            | GR-Lot-Scope-MainClassification | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-Scope] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-Scope-AdditionalClassification | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-Scope] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-ProcurementType | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-Environmental-Impact | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ProcurementType] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-Green-Procurement | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ProcurementType] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-Social-Objective | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ProcurementType] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-Innovation-Procurement | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ProcurementType] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-ProcurementType-Strategic | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ProcurementType] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-ProcurementType-Accessibility | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ProcurementType] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-PlaceOfPerformance | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-5101(a)-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-5101(b)-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-5101(c)-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-5131-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-5121-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-5071-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-5141-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
            | --- WRONG nodeId --- | BT-727-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess; ND-LotPlacePerformance' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotPlacePerformance; ND-LotPerformanceAddress'
        | GR-Lot-PlannedDuration | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-538-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotDuration'
            | --- WRONG nodeId --- | BT-536-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotDuration'
            | --- WRONG nodeId --- | BT-537-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotDuration'
            | --- WRONG nodeId --- | BT-36-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotDuration'
        | GR-Lot-ContractExtension | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-54-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-OptionsAndRenewals'
            | --- WRONG nodeId --- | BT-57-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-OptionsAndRenewals; ND-OptionsDescription'
            | --- WRONG nodeId --- | BT-58-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-OptionsAndRenewals'
        | GR-Lot-AuctionTerms | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-GpaAgreement | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-EUFunds-Indicator | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-60-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms'
        | GR-Lot-EUFunds | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-ContractingSystem | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-FrameworkAgreement-Use | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ContractingSystem] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | GR-Lot-DPS-Use | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose; GR-Lot-ContractingSystem] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
        | GR-Lot-FrameworkAgreement | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-271-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope; ND-LotValueEstimate; ND-LotValueEstimateExtension'
        | GR-Lot-FiscalLegis | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | OPT-301-Lot-FiscalLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotFiscalLegislation'
            | --- WRONG nodeId --- | OPT-110-Lot-FiscalLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotFiscalLegislation'
            | --- WRONG nodeId --- | OPT-111-Lot-FiscalLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotFiscalLegislation'
        | GR-Lot-EnvironLegis | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | OPT-301-Lot-EnvironLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotEnvironmentalLegislation'
            | --- WRONG nodeId --- | OPT-120-Lot-EnvironLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotEnvironmentalLegislation'
            | --- WRONG nodeId --- | OPT-112-Lot-EnvironLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotEnvironmentalLegislation'
        | GR-Lot-EmployLegis | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | OPT-301-Lot-EmployLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotEmploymentLegislation'
            | --- WRONG nodeId --- | OPT-130-Lot-EmployLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotEmploymentLegislation'
            | --- WRONG nodeId --- | OPT-113-Lot-EmployLegis expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotTenderingTerms; ND-LotEmploymentLegislation'
        | GR-Lot-AdditionalInformation | ContentIds: [notice-data; GR-Lot; GR-Lot-Purpose] | noticeNodeIds: [ND-Root; ND-Lot; ND-LotTenderingProcess]
            | --- WRONG nodeId --- | BT-300-Lot expected: 'ND-Root; ND-Lot; ND-LotTenderingProcess' got: 'ND-Root; ND-Lot; ND-LotProcurementScope'

I've built my code assuming that there was consistency between the information given in field.json about the xml structure and the nodeId values given in the notice-types. Maybe I made a wrong assumption?

@bertrand-lorentz bertrand-lorentz added the notice-types Related to the notice type definitions (/notice-types). label Jan 12, 2024
@bertrand-lorentz
Copy link
Contributor

You mention that you are trying to "convert a physical model into a visual model", so I assume that you're trying to prefill a form based on the information in an XML notice.

The information in the notice-types is consistent with the information on the XML structure given in fields.json is consistent for the nodes where it really matters: the repeatable nodes.
See the indication in https://docs.ted.europa.eu/eforms/latest/guide/xml-generation.html#_visiting_the_elements_in_your_visual_model

The closest repeatable parent node for "BT-60-Lot" is "ND-Lot", so the XML will only ever contain one occurence of "BT-60-Lot" in each "ND-Lot". To put the value from the XML in the right place, you just need to find the right occurence of "GR-Lot" in the form, and go the single place that corresponds to "BT-60-Lot".

When we have associated non-repeatable nodes with display groups (GR-...) in notices-types, it was to help loosely relate sections of the XML (elements that contain other elements) with what a user see on the screen, in particular to indicate when a complete section of the form has not been filled in.
As you describe, there is not an exact match, so we might wnt to look into removing some associations if they do cause confusion.

@VirtualSatai
Copy link
Author

Hi Bertrand. Thank you for quick reply, and the great work with the SDK it's a pleasure to work with! You're correct in your assumption that I'm trying to prefill a form based on the information in an XML notice.

I see your point about the repeatable nodes being the most essential to get right, and thank you for the reference to the relevant documentation. I might not have looked closed enough about the importance of repeatable vs. non-repeatable groups.

I've implemented by code by using the visitor pattern on the notice-type, and at the same time navigating through the XML following the nodeId information and the xmlStructure from fields.json. This is why the inclusion of "nodeId": "ND-LotTenderingProcess" is an issue for me, since it causes the code to assume that only the XML element related to ND-LotTenderingProcess is relevant. I can work around this by only navigating though the XML when the group is repeatable. I'll look further into if this can be a viable solution for our use-case.

Working with the eforms-notice-editor (branch: update/open-notice-with-attributes) i find that it has the same issue, if i open notice-type 16 and sdk version: 1.9 in it and generate an empty XML i get the following output in the console:

Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotContractAdditionalNature
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotMainClassification
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotAdditionalClassification
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotEnvironmentalImpactType
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotGreenCriteria
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotSocialObjectiveType
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotInnovativeAcquisitionType
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-StrategicProcurementType
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-AccessibilityJustification
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-LotPlacePerformance
Problem in visual node hierarchy, unexpected missing repeatable nodeId=ND-Lot, sdkId=ND-Funding

Removing the line "nodeId": "ND-LotTenderingProcess" from 16.json causes this issue to disappear, since it's a non-repeating group it can be omitted as you pointed out. This is why I think it might be a good idea to resolve this issue in the SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notice-types Related to the notice type definitions (/notice-types).
Projects
None yet
Development

No branches or pull requests

2 participants