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

Only forcibly generate diagram files for report if missing - use custom diagrams in report #71

Merged
merged 5 commits into from
Jun 17, 2024

Conversation

softScheck
Copy link
Contributor

@softScheck softScheck commented Apr 29, 2024

It is not possible to use a self-created diagram for the report because the creation of the diagrams is enforced, even when --generate-data-flow-diagram=0 or --generate-data-asset-diagram=0 is provided. This pull request adds a check to see if the files already exists and allows for skipping the creation of the diagrams in that case.

Therefore, a custom diagram in the build folder can be used and is build into the report.

@ezavgorodniy
Copy link
Collaborator

For reportPDF variables generateDataFlowDiagram and generateDataAssetsDiagram are always true and proposed changes will never be executed: https://github.com/Threagile/threagile/blame/master/pkg/report/generate.go#L75-L81

As far as I understand you're looking for adding some flag to not generate data flow/data assets diagrams, aren't you? I'd perhaps consider the implementation in this way: 7dbcd59 leveraging configuration file. With this implementation you'll need to point to your config.json and run the program with flag --config config.json

@softScheck
Copy link
Contributor Author

Hey,

In my pull request, I modified the generate.go file you pointed out. Specifically, I updated the section where the variables were previously set to true, regardless of the --generate-data-flow-diagram=0 and --generate-data-asset-diagram=0 flags.

Now, the code includes a check to see if the necessary files for building the report already exist. If they do and the variables are set, the generation is skipped.

This approach is more transparent than simply ignoring the --generate-data-flow-diagram=0 and --generate-data-asset-diagram=0 flags without any notification. Additionally, it allows you to place your diagrams in the build folder and have them automatically added to the report.

I think your solution using the config file should be additional to this pull request, as a config will be used to replace setting command line arguments when building the report.

@softScheck softScheck changed the title Only forcibly generate diagram files for report if missing Only forcibly generate diagram files for report if missing - use custom diagrams in report Jun 14, 2024
@ezavgorodniy
Copy link
Collaborator

Sorry, I missread the PR, my fault

Could you please rebase your branch against master branch to contain latest code changes because right now the static checks are failing?
Also could you please run go fmt against your changes to make the code more complaining with go code standards (like exclamation mark shall not have space between exclamation mark and variable)?
Or even better to move this added bit of code to the function to avoid too much nesting

@softScheck
Copy link
Contributor Author

Rebase is done and format is corrected.

@ezavgorodniy
Copy link
Collaborator

ezavgorodniy commented Jun 17, 2024

regarding errors:

@softScheck
Copy link
Contributor Author

okay, changed to the now used get methods.

@ezavgorodniy
Copy link
Collaborator

ezavgorodniy commented Jun 17, 2024

lgtm thanks for contribution

@ezavgorodniy ezavgorodniy merged commit 6ad6635 into Threagile:master Jun 17, 2024
6 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants