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

6295 add crawl function for generating reports #6347

Merged
merged 6 commits into from
Jan 30, 2025

Conversation

fergie-nz
Copy link
Contributor

Fixes #6295

πŸ‘©πŸ»β€πŸ’» What does this PR do?

  • Adds crawl function to flexibly build reports in any folder structure.
  • Also adds better error handling for anything cropping up when building reports
  • Renames manifest.json to report-manifest.json

πŸ’Œ Any notes for the reviewer?

I've also updated error handling to make diagnosing errors with report development more easeful.
I was a little unsure about when to throw an error vs when to continue with other reports which might be working fine.

πŸ§ͺ Testing

(Testing for devs - the report build cli is not accessible in testers environment)

  • Move reports into a new file structure (ie separate reports into client sub folders, move them into folders within each other)
  • See that report generation still works fine
  • Generated json file contains all reports within the nominated dir

πŸ“ƒ Documentation

  • Part of an epic: documentation will be completed for the feature as a whole

Sorry, something went wrong.

Also move super long function into utils file
@github-actions github-actions bot added this to the v2.6.0 milestone Jan 29, 2025
@github-actions github-actions bot added enhancement New feature or request Team Ruru πŸ¦‰ Roxy, Ferg, Noel feature: reports labels Jan 29, 2025
@andreievg andreievg self-assigned this Jan 29, 2025
}
(Some(arguments_path), Some(arguments_ui_path)) => Some(
schema_from_row(FormSchemaRow {
id: argument_schema_id.unwrap_or(format!("for_report_{}", id)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think format!("for_report_{}") is enough for all cases. Getting some id from local database doesn't make sense to me, reports are not database specific. Also I don't think that report making should rely on any database operations, please remove con. (It's not a new change in this PR, i understand, but would be great to get rid of it for now )

Copy link
Collaborator

@andreievg andreievg left a comment

Choose a reason for hiding this comment

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

Looks good thanks, although I haven't tested.

There is one change please, sorry it's not related to your new chagnes, but I would like it cleared up

@fergie-nz fergie-nz requested a review from andreievg January 30, 2025 00:55
Copy link
Collaborator

@andreievg andreievg left a comment

Choose a reason for hiding this comment

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

Looks great, thank !

@fergie-nz fergie-nz merged commit e3f1c1a into develop Jan 30, 2025
0 of 2 checks passed
@fergie-nz fergie-nz deleted the 6295-add-crawl-function-for-generating-reports branch January 30, 2025 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature: reports Team Ruru πŸ¦‰ Roxy, Ferg, Noel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CLI to crawl through and find all dirs containing manifest.json
2 participants