Skip to content

Add AuditCycles#478

Merged
JieXiong9119 merged 13 commits intodevelop-v2from
feat/add-audit-cycles
Jun 30, 2023
Merged

Add AuditCycles#478
JieXiong9119 merged 13 commits intodevelop-v2from
feat/add-audit-cycles

Conversation

@JieXiong9119
Copy link
Copy Markdown
Contributor

Any background context you want to provide?

FEMP CERL use case

What does this PR do?

See proposal

How should this be manually tested?

What are the relevant tickets?

#466

Screenshots (if appropriate)

@JieXiong9119 JieXiong9119 added Schema: Reports Update to reports Non-breaking Change feature Adding new functionality to BuildingSync labels Jun 12, 2023
@JieXiong9119 JieXiong9119 self-assigned this Jun 12, 2023
@markborkum
Copy link
Copy Markdown
Contributor

@JieXiong9119, Could we add more metadata elements to the new auc:AuditCycle element, e.g., the audit cycle name, the reporting juristdiction, etc. (or, dare I suggest it, an auc:UserDefinedFields child element)?

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@JieXiong9119, Could we add more metadata elements to the new auc:AuditCycle element, e.g., the audit cycle name, the reporting juristdiction, etc. (or, dare I suggest it, an auc:UserDefinedFields child element)?

Is it possible to map the cycle name to its ID? Is a Note field good for reporting jurisdiction? Sure I can add UDF as child.

@markborkum
Copy link
Copy Markdown
Contributor

Is it possible to map the cycle name to its ID?

In Audit Template, yes. A database constraint ensures that audit cycle names are unique (within the scope of a given reporting jurisdiction). At import, the tool would find the //auc:Report/auc:UserDefinedFields/auc:UserDefinedField[auc:FieldName = 'Audit Template Report Type']/auc:FieldValue element and resolve the corresponding reporting jurisdiction. Then, it is straightforward to resolve the audit cycle by its name.

Is a Note field good for reporting jurisdiction?

This is a question for CERL. Currently, the report type and audit cycle name are sufficient to identify the reporting jurisdiction and audit cycle, respectively. If additional metadata are requested by CERL, it could be exported from Audit Template using the auc:UserDefinedFields element.

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@markborkum Check this update out!

Copy link
Copy Markdown
Contributor

@markborkum markborkum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the new auc:AuditCycles element be a child of the auc:Facility element (instead of the auc:Report element)?

This way, the same audit cycle could be referenced by multiple reports in the same XML document, e.g., using a new auc:LinkedAuditCycle element:

<auc:Facility ID="facility1">
  <!-- ... -->
  <auc:AuditCycles>
    <auc:AuditCycle ID="cycle1">
      <auc:AuditCycleName>Example Audit Cycle</auc:AuditCycleName>
      <auc:AuditCycleStartYear>2023</auc:AuditCycleStartYear>
      <auc:AuditCycleEndYear>2025</auc:AuditCycleEndYear>
    </auc:AuditCycle>
  </auc:AuditCycles>
  <!-- ... -->
  <auc:Reports>
    <auc:Report ID="report1">
      <!-- ... -->
      <auc:LinkedAuditCycle IDref="cycle1">
        <auc:YearOfAuditCycle>1</auc:YearOfAuditCycle>
      </auc:LinkedAuditCycle>
      <!-- ... -->
    </auc:Report>
  </auc:Reports>
  <!-- ... -->
</auc:Facility>

Comment thread proposals/2023/Add AuditCycles.md
@JieXiong9119
Copy link
Copy Markdown
Contributor Author

Could the new auc:AuditCycles element be a child of the auc:Facility element (instead of the auc:Report element)?

This way, the same audit cycle could be referenced by multiple reports in the same XML document, e.g., using a new auc:LinkedAuditCycle element:

<auc:Facility ID="facility1">
  <!-- ... -->
  <auc:AuditCycles>
    <auc:AuditCycle ID="cycle1">
      <auc:AuditCycleName>Example Audit Cycle</auc:AuditCycleName>
      <auc:AuditCycleStartYear>2023</auc:AuditCycleStartYear>
      <auc:AuditCycleEndYear>2025</auc:AuditCycleEndYear>
    </auc:AuditCycle>
  </auc:AuditCycles>
  <!-- ... -->
  <auc:Reports>
    <auc:Report ID="report1">
      <!-- ... -->
      <auc:LinkedAuditCycle IDref="cycle1">
        <auc:YearOfAuditCycle>1</auc:YearOfAuditCycle>
      </auc:LinkedAuditCycle>
      <!-- ... -->
    </auc:Report>
  </auc:Reports>
  <!-- ... -->
</auc:Facility>

That's a good idea, but Iwould prefer to put it under auc:site (and probably auc:Building as well) instead of auc:Facility. How do you think of that?

@markborkum
Copy link
Copy Markdown
Contributor

@JieXiong9119, Being a child element of auc:Facility is preferred because it would align with the implementation for other top-level concepts that are referenced at lower levels in the XML tree. Other child elements of auc:Facility include: auc:Systems, auc:Schedules, auc:Measures, and auc:Contacts.

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@JieXiong9119, Being a child element of auc:Facility is preferred because it would align with the implementation for other top-level concepts that are referenced at lower levels in the XML tree. Other child elements of auc:Facility include: auc:Systems, auc:Schedules, auc:Measures, and auc:Contacts.

Make sense. I'll modify and update soon.

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@markborkum Check the updates!
I enable multiple AuditCycles to be referred via auc:LinkedAuditCycles\auc:LinkedAuditCycle.

@markborkum
Copy link
Copy Markdown
Contributor

@JieXiong9119, The latest changes look good, thanks.

Can we work on the xs:documentation elements next, please? The flavor text should align with that of other, preexisting XML elements in the schema.

  • AuditCycleType - We need to define the term "audit cycle". For some organizations, such as CERL or FEMP, it refers to a period of time in which multiple audits may be conducted; whereas, for others, it refers to the multi-step process of actually conducting a single audit.
  • AuditCycleName - Looking at a similarly named element, like auc:PremisesName, this should be something like "Name identifying the audit cycle."
  • AuditCycleNotes - Looking at auc:PremisesNotes, this should be something like "Details about the audit cycle."
  • AuditCycleStartYear - Looking at auc:AssessmentYear, this should be something like "Year the audit cycle starts, inclusive. (CCYY)"
  • AuditCycleEndYear - Looking at auc:AssessmentYear, this should be something like "Year the audit cycle ends, inclusive. (CCYY)"
  • LinkedAuditCycle - Looking at auc:LinkedHeatingPlantID, this should be something like "ID number of AuditCycle for this report."
  • YearOfAuditCycle - Looking at auc:YearOfManufacture, the "Year" and "YearOf" prefixes indicate that the content of the element is a xs:gYear literal. We should consider redefining the content of this element to be a xs:gYear (for consistency) or renaming this element to indicate that the content is a positive integer, e.g., IndexedYearOfAuditCycle.

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@markborkum Description updated!

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

Hey @markborkum , would you be able to review it before our deadline of BuildingSync 6/30 milestone tomorrow?

@markborkum
Copy link
Copy Markdown
Contributor

@JieXiong9119,

For some organizations, such as CERL or FEMP, it refers to a period of time in which multiple audits may be conducted; whereas, for others, it refers to the multi-step process of actually conducting a single audit.

I didn't intend for that text to be used verbatim. I meant that we need to choose one definition (most likely, the "period of time" definition, but we would need to confirm this with CERL and/or FEMP).

@markborkum
Copy link
Copy Markdown
Contributor

@JieXiong9119, To align with the documentation for other elements, can we change the definition for the AuditCycle element to: "A period of time in which multiple audits may be conducted."

Also, please update the proposal so that the examples match the renamed XML elements (e.g., replace YearOfAuditCycle with IndexYearOfAuditCycle).

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@markborkum Check out the update!

@markborkum
Copy link
Copy Markdown
Contributor

@JieXiong9119, According to GitHub, the last commit, 755fa64, was yesterday.

@JieXiong9119
Copy link
Copy Markdown
Contributor Author

@markborkum It's weird that I can see it through commits but not in the PR. I reverted the commit and pushed a new one. Can you see it now?

Copy link
Copy Markdown
Contributor

@markborkum markborkum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. Thanks, @JieXiong9119!

@JieXiong9119 JieXiong9119 merged commit f14e906 into develop-v2 Jun 30, 2023
@JieXiong9119 JieXiong9119 deleted the feat/add-audit-cycles branch June 30, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Adding new functionality to BuildingSync Non-breaking Change Schema: Reports Update to reports

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants