Update status dialog for minimum exact AT versions #1065
Conversation
There was a problem hiding this comment.
@alflennik this looks great and certainly another big step towards supporting #791 and #792!
My main hang up is with the newly named TestPlanReportStatus type. I think it may cause some confusion so would like for an alternate but if I'm alone in my thought on it, it does not have to be considered as blocking since it can be easily refactored afterwards if needed.
| counts.missing !== 0 && | ||
| counts.completed === 0 && | ||
| counts.inProgress === 0 |
There was a problem hiding this comment.
nit: assigning these boolean results to values may help with readability and future maintenance if needed.
| An existing or missing TestPlanReport that can be collected for a given | ||
| TestPlanVersion. | ||
| """ | ||
| type TestPlanReportStatus { |
There was a problem hiding this comment.
Don't think I'm a huge fan of the name TestPlanReportStatus here. The way "status" has been used for test plan reports in the past, and even how it is used to currently with other entities makes me feel like there's a chance that this may bring some confusion.
There was a problem hiding this comment.
For the name I was riffing off of the "TestPlanReportStatusDialog". I was considering several alternatives but nothing felt quite as semantic. Let me know if you think of anything better, I'm always game to improve it
| Whether the TestPlanReport is actually required during the given | ||
| TestPlanVersion phase. | ||
| """ | ||
| isRequired: Boolean! |
| @@ -1,60 +0,0 @@ | |||
| const requiredReports = { | |||
There was a problem hiding this comment.
I've wondered how well this could scale. Removing it seems like a plus!
server/resolvers/TestPlanVersion/testPlanReportStatusesResolver.js
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,179 @@ | |||
| const atResolver = require('../TestPlanReport/atResolver'); | |||
There was a problem hiding this comment.
This was a lot but the tests definitely make understanding this much easier!
…r.js Co-authored-by: Howard Edwards <[email protected]>
This includes work to support #791 and #792. Includes the following changes: * #1055 * #1001 * #1065 * #1052 * #1087 * #1098 * #1092 * #1131 * #1124 --------- Co-authored-by: Howard Edwards <[email protected]> Co-authored-by: Paul Clue <[email protected]> Co-authored-by: alflennik <[email protected]>
Updates the TestPlanStatusDialog to support minimum and exact AtVersions, which required extensive changes to the algorithm that populates the content of that dialog.
One difference worth highlighting is that I updated the sorting of the dialog to sort by AT before sorting by "required" - the dialog feels a lot more organized when all of the reports are gathered together for one AT, especially because recommended reports can have a huge number of rows in the dialog.
One limitation of this PR is the fact that the automatic switch from minimumAtVersion to exactAtVersion which occurs when a testPlanVersion goes from candidate to recommended has not yet been implemented, mostly because this PR is already bulging at the seams and also because the PR enabling it has not yet been merged into #1055. That seems like a good follow-on task to this one.