Test plan table #620
Test plan table #620
Conversation
| ); | ||
|
|
||
| return data.map(d => { | ||
| const latestTestPlanVersion = d.dataValues.testPlanVersions.sort( |
There was a problem hiding this comment.
I believe sequelize provides a way for us to also declare the order with the included associations, which may be more optimal to do in this case.
I haven’t found a clear example for this version (v6) but this link may be helpful https://sequelize.org/docs/v6/core-concepts/model-querying-basics/#ordering.
| testPlanVersionOrder: [['updatedAt', 'desc']], | ||
| testPlanVersionAttributes | ||
| const testPlans = async () => { | ||
| const plans = await getTestPlans(); |
There was a problem hiding this comment.
Just to point out that because the default TEST_PLAN_VERSION_ATTRIBUTES are will be used in the function being called, it will force unexpected columns being queried as well. Such as the tests column from TestPlanVersion, which we know causes a huge overhead when querying from the database.
Maybe you could try using this with the retrieveAttributes function which was previously used here and also used in other resolvers? It attempts to map the raw attributes requested in the GraphQL query, to relevant column names for the table(s), so that only those columns are queried for.
There was a problem hiding this comment.
This is fantastic work Erika and also thanks Howard for figuring out the initial proposal. In addition to reviewing the code and verifying that the app works as intended, this PR led to hour-long discussion with Howard and the other members of the engineering team, where I learned a lot about how the app ACTUALLY works, as opposed to how I thought it works. At the end of that I was sold that this is the right approach and more of a future-proof solution than I even realized initially. Since we now have two approvals @howard-e do you want to merge this on Erika's behalf?
This PR implements a higher level Test Plan table, which has a one-to-many relationship with both the Test Plan Version and Test Plan Report table. This PR also changes the GraphQL resolvers to use the new Test Plan model. Looking forward to the discussion that arises from this!
To use: