15766: item sent event #15920
15766: item sent event #15920
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
mkalish
force-pushed
the
platform/kalish/15766-azure-event-item-sent
branch
from
2f64dd9
to
4aac7e0
Compare
mkalish
force-pushed
the
platform/kalish/15766-azure-event-item-sent
branch
from
4aac7e0
to
689cb04
Compare
mkalish
force-pushed
the
platform/kalish/15766-azure-event-item-sent
branch
from
689cb04
to
90ada17
Compare
| @@ -616,6 +624,45 @@ class ActionHistory( | |||
| ) | |||
| } | |||
|
|
|||
| val lineages = Report.createItemLineagesFromDb(header, sentReportId) | |||
| lineages?.forEach { itemLineage -> | |||
There was a problem hiding this comment.
This algorithm is okay for now, but just thinking out loud, downloading blobs and parsing them just to get Item information EVERY TIME we need it in the various steps seems inefficient. If we were storing "Item" in the database, the bundle digest is something that would fit nicely there and then we could just get it from the DB (given it does not contain PII). Alternatively, we could also use the covid pipelines metadata table strategy probably.
I don't think any of this is worth considering until we gather the external message monitoring requirements.
There was a problem hiding this comment.
Makes sense to me. My ultimate preference would to have a clearer delineation of the uses cases since this doesn't even feel like monitoring and is more an analytics/business reporting question
| @@ -520,4 +526,33 @@ class ReportGraph( | |||
|
|
|||
| return select | |||
| } | |||
|
|
|||
| /** | |||
| * Fetches all descendant report records in a recursive manner. | |||
There was a problem hiding this comment.
"ancestor" instead of "descendant"?
| @@ -565,6 +572,7 @@ class ActionHistory( | |||
| result: String, | |||
| header: WorkflowEngine.Header, | |||
| reportEventService: IReportStreamEventService, | |||
| reportService: ReportService, | |||
There was a problem hiding this comment.
Would this be better as a class member?
There was a problem hiding this comment.
It was really messy with how the action history gets instantiated to do it that way (also why I'm passed in the reportEventService that way originally). I was bucketing this is in things to clean up if we refactor how some this works
| return reportGraph.getRootReports(childReportId).distinctBy { it.reportId } | ||
| } | ||
|
|
||
| fun getReportsForStep(childReportId: ReportId, childIndex: Int, task: TaskAction): ReportFile? { |
There was a problem hiding this comment.
kdoc? Also confusing because query only returns one but method name is getReports?
There was a problem hiding this comment.
Yep, little copy pasta; this actually fetches a specific report for an item and action. Will update the name
| @@ -213,6 +212,17 @@ class ReportGraph( | |||
| return descendantReportRecords(txn, cte, searchedForTaskActions).fetchInto(ReportFile::class.java) | |||
| } | |||
|
|
|||
| fun getAncestorReports( | |||
|
Back to you @arnejduranovic |
|
This PR adds a new ITEM_SENT azure event
Test Steps:
ITEM_SENTevent is generated for each item in the delivered report and that the data is correct, specifically looking at the childItemIndex and submittedItemIndexChanges
Checklist
Testing
./prime testor./gradlew testSmokeagainst local Docker ReportStream container?npm run lint:write?Process
Linked Issues
To Be Done
Specific Security-related subjects a reviewer should pay specific attention to