fix: Site build polling and latest build branch #4663
Conversation
🤖 This is an automated code coverage reportTotal coverage (lines): 14.04% |
| import { useQuery } from '@tanstack/react-query'; | ||
| import api from '@util/federalistApi'; | ||
|
|
||
| // Assumes the order of builds is based on createdAt desc |
There was a problem hiding this comment.
I think there's a slight potential for race conditions because of createdAt sorting vs completedAt checking but I'm fine ignoring for now
frontend/support/data/builds.js
Outdated
| @@ -0,0 +1,1349 @@ | |||
| export const builds = [ | |||
There was a problem hiding this comment.
Eventually we should create some standardized way of creating our fixtures but this is perfect for now
| import AlertBanner from '@shared/alertBanner'; | ||
| import LoadingIndicator from '@shared/LoadingIndicator'; | ||
|
|
||
| export default function QueryPage({ |
| /> | ||
| </div> | ||
| )} | ||
| {data?.length > 0 && ( |
There was a problem hiding this comment.
Is there a way to avoid these checks in the QueryPage children? It shouldn't try to render the child without data but not sure if component still gets fully run
There was a problem hiding this comment.
This could be cut because the QueryPage does not render the child without data.
| return request(`site/${site.id}/build`); | ||
| return request( | ||
| `site/${site.id}/build`, | ||
| {}, |
There was a problem hiding this comment.
What are theses changes need to the API request?
There was a problem hiding this comment.
The request method kicks off a notification redux action by default and these args skips the notification action. The QueryPage wrapper we now handle the error from a react-query request with an alert.
apburnes
force-pushed
the
fix-build-polling-and-latest-build-branch
branch
2 times, most recently
from
d923955
to
88e23a9
Compare
Signed-off-by: Andrew Burnes <andrew.burnes@gsa.gov>
apburnes
force-pushed
the
fix-build-polling-and-latest-build-branch
branch
from
88e23a9
to
5f03ab4
Compare
| }); | ||
| } | ||
|
|
||
| export function useBuilds(id) { |
There was a problem hiding this comment.
I'd rename this variable siteId for clarity (and it will bind the second variable of the mutation function which I think right now might not be needed)
|
|
||
| it('should mutate the builds on rebuild', async () => { | ||
| const siteId = 1; | ||
| await getSiteBuilds(siteId, 'Something happened'); |
Closes #4650 and #4648
Changes proposed in this pull request:
security considerations
Adds new dependency react-query to manage http calls