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

Change Report Preview into Build Report tab #512

Merged
merged 13 commits into from
May 8, 2023

Conversation

AARON-CLARK
Copy link
Collaborator

@AARON-CLARK AARON-CLARK commented Apr 28, 2023

Closes #348.

The thinking here is the report preview tab is not just for visualizing the report, but it's also for configuring and capturing other important information that orgs may want to keep track of (in one place - the app). This solution offers a long form "summary" section where users can fill it up with whatever content they want. Not just content about riskmetric metrics. "Overall Comment" is still nice to have, but it is intended to be short hand.

Currently, this PR is a work-in-progress but includes a fully functional "module" to allowing the creation of "pkg_summary" comments in the db (abbreviation = "s"). If you test, you will see that "Submit" and "edit" buttons appear and disappear as needed. Also, once submitted, the text box is disabled until "Edit" is clicked. Below, you can see that I made the Report contain headers just like the HTML report, and put a shaded box and border around the div so that it really pops and looks like a page.

TODO:

  • Add pkg summary to the reports
  • Introduce markdown editor into Build Report tab this is on hold
  • Define report download configs so users are empowered to include what's important to them
    • Impact report preview - this is trickier than I thought
    • Impact reports
  • Make sure those configs are shared globally so the both download handlers use them by default

Would love to think of a way to create a reporting template that others can use

image

@codecov
Copy link

codecov bot commented May 3, 2023

Codecov Report

Merging #512 (ab13758) into dev (a0968c6) will increase coverage by 1.16%.
The diff coverage is 72.43%.

❗ Current head ab13758 differs from pull request most recent head 5e51e42. Consider uploading reports for the commit 5e51e42 to get more accurate results

@@            Coverage Diff             @@
##              dev     #512      +/-   ##
==========================================
+ Coverage   82.51%   83.68%   +1.16%     
==========================================
  Files          25       25              
  Lines        2826     2776      -50     
==========================================
- Hits         2332     2323       -9     
+ Misses        494      453      -41     
Impacted Files Coverage Δ
R/mod_reportPreview.R 75.11% <67.30%> (-20.47%) ⬇️
R/app_ui.R 100.00% <100.00%> (ø)
R/mod_databaseView.R 39.20% <100.00%> (-5.33%) ⬇️
R/mod_downloadHandler.R 64.96% <100.00%> (+6.51%) ⬆️
R/mod_viewComments.R 100.00% <100.00%> (ø)
R/utils.R 96.75% <100.00%> (ø)
R/utils_get_db.R 94.59% <100.00%> (+0.07%) ⬆️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AARON-CLARK AARON-CLARK marked this pull request as ready for review May 3, 2023 13:22
@AARON-CLARK
Copy link
Collaborator Author

AARON-CLARK commented May 3, 2023

FYI - I wanted to implement a markdown editor instead of a boring textAreaInput() (like shinymarkdown) to produce something github-like the editor seen below, but alas - the package doesn't seem to work well with shiny modules. I opened an issue but we should probably use something that's on CRAN anyway.

image

@AARON-CLARK
Copy link
Collaborator Author

@ScottSchumacker & @Robert-Krajcik, I fixed my merge conflicts w/ dev. Should be ready for review now. Thanks!

Copy link
Contributor

@Robert-Krajcik Robert-Krajcik 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 to Me, @AARON-CLARK

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.

2 participants