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

eForms SDK 1.8.0 #188

Closed
jpmckinney opened this issue Jul 26, 2023 · 18 comments
Closed

eForms SDK 1.8.0 #188

jpmckinney opened this issue Jul 26, 2023 · 18 comments
Assignees

Comments

@jpmckinney
Copy link
Member

jpmckinney commented Jul 26, 2023

Release notes: https://github.com/OP-TED/eForms-SDK/blob/develop/CHANGELOG.md

Changes requiring updates

  • Add OPT-211-Tenderer
  • Remove
    • OPA-118-NoticeResult-Currency (guidance was Discard)
    • OPA-161-NoticeResult-Currency (guidance was Discard)
    • OPA-27-Procedure-Currency (guidance linked to BT-27-Procedure)
    • OPT-050-Lot (guidance was Discard with link to BT-708-Lot or BT-737-Lot)
    • OPT-050-Part (guidance was Discard with link to BT-708-Part or BT-737-Part)
  • Split BT-752 in two
    • Add BT-752-Lot-ThresholdNumber and BT-752-Lot-WeightNumber
    • Remove BT-752-Lot (left in guidance.yaml until additions updated)
  • Split BT-541 into 6 fields, instead of 2 ("Added nodes for award criteria and created six different fields for BT-541 instead of two fields; also leading to updates for related rules, unpublished fields and GR-Lot-AwardCriteria-Criterion-Parameter.")
    • Add:
      • BT-541-Lot-FixedNumber
      • BT-541-Lot-ThresholdNumber
      • BT-541-Lot-WeightNumber
      • BT-541-LotsGroup-FixedNumber
      • BT-541-LotsGroup-ThresholdNumber
      • BT-541-LotsGroup-WeightNumber
      • BT-195(BT-541)-Lot-Fixed
      • BT-195(BT-541)-Lot-Threshold
      • BT-195(BT-541)-Lot-Weight
      • BT-195(BT-541)-LotsGroup-Fixed
      • BT-195(BT-541)-LotsGroup-Threshold
      • BT-195(BT-541)-LotsGroup-Weight
      • And the 6 for BT-196, BT-197, BT-198
    • Remove: (left in guidance.yaml until additions updated)
      • BT-541-Lot
      • BT-541-LotsGroup
      • BT-195(BT-541)-Lot
      • BT-195(BT-541)-LotsGroup
      • And the pairs for BT-196, BT-197, BT-198

It also added a bunch of fields for XML attributes, but I think we already map these at the level of the XML element, so I changed manage.py to not import them into guidance.yaml.

Other changes that maybe affect mapping

I don't know if repeatable or mandatory affect mapping, so noting here.

  • Many fields become mandatory
  • Fields become optional
    • BT-67(b)-Procedure
    • BT-36-Lot
  • Fields become non-repeatable
    • BT-1375-Procedure
    • BT-191-Tender
    • BT-3202-Contract
    • BT-46-Lot
    • BT-47-Lot
    • BT-501-Organization-Company
    • BT-531-Lot
    • BT-531-Part
    • BT-531-Procedure
    • BT-651-Lot
    • BT-706-UBO
    • BT-774-Lot
    • BT-775-Lot
    • BT-776-Lot
    • BT-97-Lot
    • OPP-040-Procedure
    • OPP-090-Procedure
    • OPT-300-Contract-Signatory
    • OPT-301-LotResult-Financing
    • OPT-301-LotResult-Paying
    • OPT-301-Tenderer-MainCont
    • OPT-302-Organization
    • OPT-315-LotResult
    • OPT-320-LotResult

Other changes that don't affect mapping

  • Change parentNodeId
  • Change pattern for URLs
  • Simplify xpathAbsolute (hierarchy is the same, but removed repetitive selectors, added or moved some selectors)
@odscjen
Copy link
Contributor

odscjen commented Jul 28, 2023

OPT-211 (Tendering party name) - Guidance to discard similar as for BT-210 which is the tendering party ID
BT-752 removed and guidance added for new splits
(then my brain gave up for the week so didn't get any further)

mandatory doesn't affect any of the mappings but I think repeatable does.

@odscjen
Copy link
Contributor

odscjen commented Jul 31, 2023

All BT-541 related fields now updated and old fields removed.

I checked through the now non-repeatable fields, only 1 needed updated (OPP-090 - removing a sentence about if the field was repeated).

We created new extension fields for 2 which we made array's due to the BT's being repeatable but now they're not:

In both cases I don't think we should bother changing these from arrays to strings.

@jpmckinney
Copy link
Member Author

Hmm, those two BTs are repeatable in the regulation.

This led me to dig deeper. It turns out: If a repeatable business term is implemented as a field that is the only child of a parent, then the SDK marks the parent as repeatable, rather than the child.

I've updated the logic to have a field inherit repeatable from its parent if the field is an only child.

This causes the following to become repeatable, including OPP-090.

Field Name
BT-137-Lot Purpose Lot Identifier
BT-137-LotsGroup Purpose Lot Identifier
BT-137-Part Purpose Lot Identifier
BT-13716-notice Change Previous Section Identifier
BT-1375-Procedure Group Lot Identifier
BT-1501(n)-Contract Modification Previous Notice Identifier
BT-1501(s)-Contract Modification Previous Notice Section Identifier
BT-191-Tender Country Origin
BT-202-Contract Modification Description
BT-3202-Contract Contract Tender ID (Reference)
BT-330-Procedure Group Identifier
BT-46-Lot Jury Member Name
BT-47-Lot Participant Name
BT-501-Organization-Company Organisation Identifier
BT-531-Lot Additional Nature (different from Main)
BT-531-Part Additional Nature (different from Main)
BT-531-Procedure Additional Nature (different from Main)
BT-651-Lot Subcontracting Tender Indication
BT-706-UBO Winner Owner Nationality
BT-708-Lot Documents Official Language
BT-708-Part Documents Official Language
BT-71-Lot Reserved Participation
BT-71-Part Reserved Participation
BT-723-LotResult Vehicle Category
BT-728-Lot Place Performance Additional Information
BT-728-Part Place Performance Additional Information
BT-728-Procedure Place Performance Additional Information
BT-735-Lot CVD Contract Type
BT-735-LotResult CVD Contract Type
BT-737-Lot Documents Unofficial Language
BT-737-Part Documents Unofficial Language
BT-774-Lot Green Procurement
BT-775-Lot Social Procurement
BT-776-Lot Procurement of Innovation
BT-805-Lot Green Procurement Criteria
BT-97-Lot Submission Language
OPP-040-Procedure Main Nature - Sub Type
OPP-090-Procedure Previous Notice Identifier
OPT-030-Procedure-SProvider Provided Service Type
OPT-300-Contract-Signatory Signatory Identifier Reference
OPT-301-LotResult-Financing Financing Party (ID reference)
OPT-301-LotResult-Paying Payer Party (ID reference)
OPT-301-Tenderer-MainCont Main Contractor ID Reference
OPT-301-Tenderer-SubCont Subcontractor ID Reference
OPT-302-Organization Beneficial Owner Reference
OPT-315-LotResult Contract Identifier Reference
OPT-320-LotResult Tender Identifier Reference

@odscjen
Copy link
Contributor

odscjen commented Aug 3, 2023

Ah. So I checked through the mapping for all of this new list and the following have issues:

  • BT-137-Part - guidance maps to tender.id but obviously there can be only 1 tender.id. I think we agreed to treat 'Part' as synonymous with Procedure and create a new OCID for each part, but this doesn't appear to be specified anywhere in the guidance
  • BT-501-Organization-Company - guidance maps to parties.identifier but .identifier is an object not an array of objects as it's for the primary identifier. eForms doesn't appear to have the concept of a primary external identifier. We could change the guidance to map all of these to .additionalIdentifiers or add a line "Map any additional identifiers to `.additionalIdentifiers" and make the first instance of BT-501-Organization-Company the default primary identifier.
  • BT-706-UBO - guidance maps to beneficialOwners.nationality which is a string. eForms allows efac:Nationality to be repeatable but it's only child cbc:NationalityID (BT-706-UBO) isn't. So I'm reading this as multiple nationalities are allowed (and people can have multiple nationalities) which OCDS doesn't currently cover. We'd need to update Beneficial Owners nationality to nationalities, an array of strings.
  • BT-737-Lot/Part - bigger problems that just repeatability with this one, see BT-708 and BT-737 official and nonofficial languages in tender documents #189

All the others seem fine as they either map to array's or the guidance specifies to create a new object for each repeat.

@jpmckinney
Copy link
Member Author

  • BT-137-Part: It's repeatable only for prior information notices (eForms notices 4, 5 and 6), like in TED XML. eForms makes the repeatability conditional by adding constraints. If we don't have guidance on when to mint OCIDs, we'll need content like this from the old profile: https://standard.open-contracting.org/profiles/eu/latest/en/operations/#create-a-release
  • BT-501-Organization-Company: Let's put them all as additionalIdentifiers
  • BT-706-UBO: Let's make that change.

I'll look at #189

@jpmckinney
Copy link
Member Author

Making a note that I should compare with the Repeatable column in the regulation's annex, since the annex is correct at the BT level – to double-check that we haven't missed any others at the field level.

@odscjen
Copy link
Contributor

odscjen commented Aug 3, 2023

If we don't have guidance on when to mint OCIDs, we'll need content like this from the old profile: https://standard.open-contracting.org/profiles/eu/latest/en/operations/#create-a-release

We do have in operations.md

Create a release

  1. Set id to the notice number (/*/cbc:ID).
  2. Set initiationType to 'tender'.
  3. Set ocid as described below.

The notice's ocid will either be a new ocid, or the same ocid as the previous publication concerning this procedure. The notice's ocid will be a new ocid if one of the following is true:

  • The notice is the first publication concerning the procedure.
  • The previous publication is a prior information notice or a periodic indicative notice (PIN) that has multiple /*/cac:ProcurementProjectLot elements, it potentially lead to the launch of several procedures, each with its own ocid.
  • The notice is a contract award notice (CAN) for an award within a framework agreement or dynamic purchasing system.

If none is true, then set the notice's ocid to be the same as the previous publication's ocid. Otherwise, set the notice's ocid by prepending your OCID prefix to a unique identifier of your choice (e.g. a version 4 UUID or a suitable system-internal identifier).

Which is a bit vague on what to do in the case of "a periodic indicative notice (PIN) that has multiple /*/cac:ProcurementProjectLot elements"

@odscjen
Copy link
Contributor

odscjen commented Aug 3, 2023

I've updated guidance.md for BT-706-UBO and BT-501-Organization-Company.

Is it better to add nationalities as a new field in the Beneficial Owners extension or change the existing nationality field?

@jpmckinney
Copy link
Member Author

jpmckinney commented Aug 3, 2023

Feel free to reword the Create a release section to make it clearer.

@jpmckinney
Copy link
Member Author

Let's just change the field - the beneficial owners extension is not in use. cc @yolile

@jpmckinney
Copy link
Member Author

jpmckinney commented Aug 3, 2023

Okay, so now I've compared the regulation annex to the SDK fields (and the update-with-annex command will now report cases where they differ). The following from my earlier comment are actually not repeatable. There are another 17 differences reported that I need to look into.

ID Name
BT-137-Lot Purpose Lot Identifier
BT-137-LotsGroup Purpose Lot Identifier
BT-137-Part Purpose Lot Identifier
BT-202-Contract Modification Description
BT-330-Procedure Group Identifier
BT-723-LotResult Vehicle Category
BT-728-Lot Place Performance Additional Information
BT-728-Part Place Performance Additional Information
BT-728-Procedure Place Performance Additional Information
BT-735-LotResult CVD Contract Type

I need to check BT-735-Lot, which is still showing up as repeatable, though it's not so in the annex.

Also: BT-1501 is repeatable in the annex, but one of its fields in eForms (BT-1501(n)-Contract) is non-repeatable, while the other (BT-1501(s)-Contract) is repeatable. I'll assume eForms is correct, here. Edit: In 1.11.0 (s) was split into (c) and (p). (p) is repeatable, but (c) is not.

Applying the same logic as I used to find the above, these fields that aren't in the annex are actually not repeatable:

ID Name
OPT-030-Procedure-SProvider Provided Service Type
OPT-301-Tenderer-SubCont Subcontractor ID Reference

@jpmckinney
Copy link
Member Author

jpmckinney commented Aug 3, 2023

The following are repeatable in the annex but not eForms. This seems to be deliberate by eForms, but I'll try to find out: OP-TED/eForms-SDK#620

(Adding check marks if answered by OP)

  • ✅ BT-124-Lot, BT-124-Part Tool: Atypical URL
  • ✅ BT-1252-Procedure: Direct Award Justification Previous Procedure Identifier
  • ✅ BT-136-Procedure: Direct Award Justification Code
  • ✅ BT-556: Group Framework Value Lot Identifier
  • ✅ BT-65-Lot: Subcontracting Obligation
  • ✅ BT-754-Lot: Accessibility

False positives:

  • BT-702(a)-notice (Notice Official Language) is non-repeatable in eForms, because eForms adds BT-702(b)-notice for repeats, presumably because of how the XML standards it adopts are structured.
  • BT-26(a)-* (Classification Type (e.g. CPV)) are repeatable in eForms, because these are the additional classification types (for BT-263), whereas BT-26(m)-* are the main classification types (for BT-262).

Non-repeatable in the annex, but repeatable in eForms, in what seems to be a deliberate way:

  • BT-26(a)-Lot, BT-26(a)-Part, BT-26(a)-Procedure: Classification Type (e.g. CPV): I assume it was made repeatable, so that the additional codes in BT-263 (Additional Classification Codes) could have different types and not all the same type.

✅ I'm not sure if perhaps there's an issue in eForms that explains some of the other differences (the existence of BT-125(i)-Lot causes BT-1251 to be non-repeatable by our logic): OP-TED/eForms-SDK#619 (comment)


I haven't fully explored:

  • ✅ BT-1501(n)-Contract: Modification Previous Notice Identifier (repeatable in annex)
  • BT-735-Lot: CVD Contract Type (non-repeatable in annex)
    • Edit: In the Annex, it has siblings, but in eForms, the siblings are moved to have different parents. Given that its now an only child, from the Annex's perspective, this would mean the BG's repeatability can be inherited by the single child.

@odscjen
Copy link
Contributor

odscjen commented Aug 7, 2023

@duncandewhurst @jpmckinney how is this as a new version of Create a release to make it clearer how publishers should deal with multiple Parts in PINs

Create a release

  1. Set id to the notice number (/*/cbc:ID).
  2. Set initiationType to 'tender'.
  3. Set ocid as described below.

The notice's ocid will either be a new ocid, or the same ocid as the previous publication concerning this procedure. The notice's ocid will be a new ocid if one of the following is true:

  • The notice is the first publication concerning the procedure.
  • The notice is a contract award notice (CAN) for an award within a framework agreement or dynamic purchasing system.
  • The publication is a prior information notice or a periodic indicative notice (PIN) that has multiple /*/cac:ProcurementProjectLot elements where schemeName="Part".

If none is true, then set the notice's ocid to be the same as the previous publication's ocid. Otherwise, set the notice's ocid by prepending your OCID prefix to a unique identifier of your choice (e.g. a version 4 UUID or a suitable system-internal identifier). If the notice is a PIN with multiple /*/cac:ProcurementProjectLot elements where schemeName="Part", this must be done for each cac:ProcurementProjectLot/cbc:ID schemeName="Part"/cbc:ID.

If the previous notice is a PIN with multiple /*/cac:ProcurementProjectLot elements where schemeName="Part" set the notice's ocid to be the same as the ocid created for the relevant Part in the PIN.

If the notice is a contract award notice for an award within a framework agreement or dynamic purchasing system, you must also add a RelatedProcess object to the relatedProcesses array, set its .id to '1', add 'framework' to its .relationship array, set its .scheme to 'ocid', and set its .identifier to the ocid of the procedure that set up the framework agreement or dynamic purchasing system.

@duncandewhurst
Copy link
Contributor

duncandewhurst commented Aug 10, 2023

If the previous notice is a PIN with multiple /*/cac:ProcurementProjectLot elements where schemeName="Part" set the notice's ocid to be the same as the ocid created for the relevant Part in the PIN.

I don't think we need this part because we treat PIN only notices as planning processes so subsequent notices are assigned new identifiers as explained in this Slack message.

If the notice is a contract award notice for an award within a framework agreement or dynamic purchasing system, you must also add a RelatedProcess object to the relatedProcesses array, set its .id to '1', add 'framework' to its .relationship array, set its .scheme to 'ocid', and set its .identifier to the ocid of the procedure that set up the framework agreement or dynamic purchasing system.

I don't think we need this part because it is covered by the mapping for OPT-100-Contract.


I've taken a pass at redrafting and adding some explanation of the treatment of PIN only and subsequent notices below

The only change in logic from the existing guidance is to always assign a new ocid when the previous publication is a PIN only notice. That change is based on the considerations in https://docs.ted.europa.eu/eforms/latest/schema/procedure-lot-part-information.html#previousPlanningSection, which state that a lot may be defined based on multiple parts, and parts may belong to different "PIN only" notices.

I am not sure whether it is OK to refer to 'planning process' in the introductory sentence or whether it should still be 'contracting process' until OCDS 1.2 is released.

Create a release

If the notice is a prior information or periodic indicative notice used only for information (PIN only), you should repeat the following steps for each part (/*/cac:ProcurementProjectLot[cbc:ID/@schemeName='Part']) because each part is treated as a separate planning process. Otherwise, you need only perform the steps once per notice.

  1. Create an empty JSON object
  2. Set its id to the notice identifier (/*/cbc:ID).
  3. Set its initiationType to 'tender'.
  4. Set its ocid as described below.

If any of the following are true, assign a new ocid by prepending your OCID prefix to a unique identifier of your choice (e.g. a version 4 UUID or a suitable system-internal identifier):

  • The notice is the first publication concerning the procedure.
  • The notice is a contract award notice (CAN) for an award within a framework agreement or dynamic purchasing system.
  • The previous publication concerning the procedure is a PIN only notice. Notices following a PIN only notice are assigned a new ocid because they may be defined based on multiple parts.

Otherwise, set ocid to the same value as the previous publication's ocid.

@jpmckinney
Copy link
Member Author

I think it's fine to refer to planning process here.

@duncandewhurst
Copy link
Contributor

@jpmckinney I'm assuming that you were happy with the rest of the content too so I've updated operations.md.

@jpmckinney
Copy link
Member Author

Sounds good - if the other items in the issue description are resolved (where needed), feel free to close.

@odscjen odscjen self-assigned this Aug 29, 2023
@odscjen
Copy link
Contributor

odscjen commented Sep 11, 2023

I've added the line about OPP-090 being repeatable back into guidance.yaml and double checked the rest, they're all resolved so I'm closing this now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants