Fix H1 title for test plan review page #816
Conversation
There was a problem hiding this comment.
Approved, assuming you remove that one console log. I verified it is working before / after, and I ran the tests successfully.
Code changes look good. I actually checked the code pretty carefully for this one, because I noticed that you added the first database-related call in this file. I wanted to make sure there isn't an important reason that database calls haven't yet been used. I found nothing that gives me doubt, so I do think the way you did this is the simplest and most straightforward approach.
server/apps/test-review.js
Outdated
| @@ -342,6 +345,9 @@ const generateTests = async (pattern, commit, commitDate) => { | |||
| app.get('/:commit/:pattern', async (req, res) => { | |||
| const commit = req.params.commit; | |||
| const pattern = req.params.pattern; | |||
| const testPlan = await getTestPlanById(pattern); | |||
| const testPlanTitle = testPlan.title; | |||
| // console.log("New Log", testPlan.title); | |||
There was a problem hiding this comment.
Could you remove this line?
There was a problem hiding this comment.
I am also going to approve. Title renders correctly and the DB query is necessary.
I do think that getTestPlanId (and by extension getTestPlans) could stand to be improved so that we can specify columns for retrieval. In this instance, we only need one property but we end up with the result of SELECT * FROM which is actually a deep object. This is outside the bounds of this PR though.
Good job!
see issue #789
This pull request changes the H1 value in the test plan review page from the name of the test plan directory to the actual title of the the test plan.