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.

Sign up for GitHub

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

Jump to bottom

Fix/reuse report ooi entities #4047

Merged
merged 141 commits into from
Feb 4, 2025
Merged

Fix/reuse report ooi entities #4047

merged 141 commits into from
Feb 4, 2025

Conversation

Donnype
Copy link
Contributor

@Donnype Donnype commented Jan 24, 2025

Changes

This PR changes the data structure around reporting. The main changes have been described in #3729, but this also includes:

  • A new Bytes API endpoint where we can request multiple raw files at once
  • An Octopoes API update where we now add scan profiles to the load_bulk endpoint for OOIs

Note that the tests are still a work in progress in Rocky!

Issue link

Closes #3729

Demo

Scherm­afbeelding 2025-01-24 om 13 20 56 Scherm­afbeelding 2025-01-24 om 13 21 31 Scherm­afbeelding 2025-01-24 om 13 21 47

QA notes

The main changes are around reporting, so please verify that:

  • Scheduling (aggregate) reports still works properly, especially given the other updates around schedule once
  • The report overview pages still work
  • You see the right amount of asset reports
  • You can render both aggregate, concatenated, and single asset reports
  • Of course, that the jobs still run properly given the changes in Octopoes and Bytes!

Code Checklist

  • All the commits in this PR are properly PGP-signed and verified.
  • This PR only contains functionality relevant to the issue.
  • I have written unit tests for the changes or fixes I made.
  • I have checked the documentation and made changes where necessary.
  • I have performed a self-review of my code and refactored it to the best of my abilities.
  • Tickets have been created for newly discovered issues.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Donnype and others added 30 commits December 5, 2024 15:54
@Donnype
Add a recipe ID filter in the report API
e2c1dfc
@Donnype
Introduce asset reports
f8d7b44 
WIP fix integration tests for octopoes
@Donnype
Finish Report API update
bbaeee7
@Donnype
Fix report page bug: connector sends empty recipe_id
0283fb0
@Donnype
Fix report generation through worker
5a18202
@Donnype
Add reference date as TODO
f959131
@Donnype
Some field name changes and aggregate report fixes
633dded
@Donnype
Fix format_reference_human_readable for Report, HydratedReport and As…
9590d1d 
…setReport
@Donnype
Move some code
034b3fd
@madelondohmen
Fixes on history page
f366a86
@Donnype
Remove parent/child and report_id references for multi-report
ceddc13
@Donnype
Merge remote-tracking branch 'origin/fix/reuse-report-ooi-entities' i…
8456b31 
…nto fix/reuse-report-ooi-entities
@Donnype
Add report naming docs
01ad906
@Donnype
Fix get report endpoint
958cbbf
@madelondohmen
Fix report recipe variable names
d37a403
@Donnype
Test and fix the newly implemented get_reports endpoint
f58adcd 
Update references to report_types where we actually now new asset_report_types
Style
@Donnype
Rename subreport to asset_report
94d78fc
@Donnype
Refactor report naming
e4c5231
@Donnype
Refactor aggregate report naming
6cc6f8f
@madelondohmen
Fix generation of reports and merge conflicts
9c9ef03
@madelondohmen
Fix naming in SchedulerView
c993c84
@madelondohmen
Fix tests
afa86b9
@madelondohmen
Small fixes
4137e84
@Donnype
Strict report name formats
360fdfe
@madelondohmen
Make asset name format visible in export settings
a210f79
@madelondohmen
Change children to assets
e44aa70
@Rieven
Merge branch 'main' of github.com:minvws/nl-kat-coordination into fix…
0c6454f 
…/reuse-report-ooi-entities
@Rieven
Merge branch 'main' of github.com:minvws/nl-kat-coordination into fix…
e9eb41b 
…/reuse-report-ooi-entities
@madelondohmen
Fix report overview for single reports
5da5251
@madelondohmen
857e6a5
@Donnype Donnype requested a review from a team as a code owner January 24, 2025 12:24
@Donnype Donnype added the report label Jan 24, 2025
underdarknl
underdarknl reviewed Jan 29, 2025
View reviewed changes
octopoes/octopoes/repositories/ooi_repository.py
if not references:
return []

query = Query().where_in(OOI, id=references).pull(OOI, fields="[* {:_reference [*]}]")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather see the extra referenced objects loaded optionally. If you look at the queries and datasets that we encode/decode and transport over the various api's it becomes clear we need to be much more precise in our resultsets as in many cases we don't use most of what we return.

Rieven and others added 16 commits January 29, 2025 11:19
@Rieven
fix scheduler rerun for once
499d365
@Rieven
Merge branch 'fix/report-flow' of github.com:minvws/nl-kat-coordinati…
b4104f7 
…on into fix/reuse-report-ooi-entities
@Donnype
Fix hash typo for caching
24938b5
@Donnype
Merge branch 'fix/reuse-report-ooi-entities' of github.com:minvws/nl-…
7b1d7a6 
…kat-coordination into fix/reuse-report-ooi-entities
@Donnype
Fix a lot of tests
88dc4d1
@madelondohmen
Fix rename for asset report
695290b
@madelondohmen
Fix rename for asset report - part 2
df8e323
@madelondohmen
Remove asset report name format completely
cb47062
@Donnype
Fix all unit tests
f0a729a
@Donnype
Merge remote-tracking branch 'origin/fix/reuse-report-ooi-entities' i…
0ae86f8 
…nto fix/reuse-report-ooi-entities

# Conflicts:
#	rocky/reports/templates/report_overview/report_history_table.html
@Donnype
Fix all unit tests again
64796af
@Rieven
Merge branch 'main' of github.com:minvws/nl-kat-coordination into fix…
b249db4 
…/reuse-report-ooi-entities
@Rieven
fix lang
8da8589
@Donnype
Fix integration tests
fa26378 
Style
@Donnype
Fix unit tests that caught bug for id filtering for queries that don'…
e94ff85 
…t use multiple values
@Donnype
Mypy fixes
1f0bbb0
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Feb 4, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
54.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@underdarknl underdarknl merged commit 9ab55e4 into main Feb 4, 2025
38 of 39 checks passed
@underdarknl underdarknl deleted the fix/reuse-report-ooi-entities branch February 4, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@underdarknl underdarknl underdarknl approved these changes

Assignees
No one assigned
Labels
report
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[EPIC] We should use a ReportOOI entity per Report, or update a Report in-place and traverse the history API
5 participants
@Donnype @underdarknl @madelondohmen @Rieven @jpbruinsslot