Got the drawers working again to reflect changes from invalidated queries #23
Conversation
In some cases I fell back to code from last week, leaving newer code that was not fully working in place, but commented out. At this point every ReactQuery call works perfectly (without overriding the config values set in App.jsx) and I tested them many many times. In TeamMemberList, I revived last week's working code, and left in place the code that uses apiCache. In TeamMemberList I now do a diff to show the differences from data straight out of ReactQuery (which has matched SQL every time that I have checked it manually), with the processed data that is stored in apiCache -- there currently is a discrepancy in apiDataCache. That diff should be useful in the future, and could be generalized. I did not test or fix the Questionnaire drawer yet. This is an incremental checkin, and I'll now work on getting the apiDataCache working as it exists. (Although it would be my strong preference to get it working by using ReactQuery's internal cache as the only cache for unaltered API results, and to generate the processed output through what is now apiDataCache from the ReactQuery's internal cache.)
In some cases I fell back to code from last week, leaving newer code that was not fully working in place, but commented out. At this point every ReactQuery call works perfectly (without overriding the config values set in App.jsx) and I tested them many many times. In TeamMemberList, I revived last week's working code, and left in place the code that uses apiCache. In TeamMemberList I now do a diff to show the differences from data straight out of ReactQuery (which has matched SQL every time that I have checked it manually), with the processed data that is stored in apiCache -- there currently is a discrepancy in apiDataCache. That diff should be useful in the future, and could be generalized. I did not test or fix the Questionnaire drawer yet. This is an incremental checkin, and I'll now work on getting the apiDataCache working as it exists. (Although it would be my strong preference to get it working by using ReactQuery's internal cache as the only cache for unaltered API results, and to generate the processed output through what is now apiDataCache, from data that is immediately pulled from ReactQuery's internal cache.)
There was a problem hiding this comment.
Hi @SailingSteve I look forward to coming up with a plan together this morning, for next steps.
|
|
||
|
|
||
| const EditQuestionnaireDrawer = () => { | ||
| renderLog('EditQuestionnaireDrawer'); | ||
| const { getAppContextValue } = useConnectAppContext(); | ||
|
|
||
| const selected = getAppContextValue('selectedQuestionnaire'); | ||
| const selected = getAppContextValue('selectedQuestion'); |
There was a problem hiding this comment.
This needs to be the Questionnaire as opposed to one Question within the Questionnaire.
There was a problem hiding this comment.
Yeah, don't see how my change could have worked. I might have done a reversion and then cherry picked your changes into it. I'll look into it
| </> | ||
| )} | ||
| <Button | ||
| classes={{ root: classes.addTeamMemberButtonRoot }} | ||
| color="primary" | ||
| variant="outlined" | ||
| onClick={addTeamMemberClick} | ||
| sx={{ marginTop: '30px' }} |
There was a problem hiding this comment.
I'd like to keep all of the styles in one place, and move this "marginTop" style down to the 'addTeamMemberButtonRoot' definition near the bottom of this file.
There was a problem hiding this comment.
Good idea and No problem. That seemed like a temporary layout
|
|
||
| const teamListRetrieveResults = useFetchData(['team-list-retrieve'], {}, METHOD.GET); | ||
| useEffect(() => { |
There was a problem hiding this comment.
While we are comparing approaches, please comment out instead of deleting the alternative so it is easier to discuss.
There was a problem hiding this comment.
Yes, I tired to do that, again a reversion and cherry pick mistake.
| // showHeaderLabels={(index === 0) && showAllTeamMembers && (team.teamMemberList && team.teamMemberList.length > 0)} | ||
| showIcons | ||
| /> | ||
| <TeamHeader team={team} showHeaderLabels={(index === 0) && showAllTeamMembers && (team.teamMemberList && team.teamMemberList.length > 0)} showIcons /> |
There was a problem hiding this comment.
The rest of the team has standardized on putting each prop being passed into a component on its own line when we have 3+ props. It makes it easier to see what is being passed in at a glance.
There was a problem hiding this comment.
No problem, again a reversion/cherry pick
| const queryClient = new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { | ||
| networkMode: 'always', // Send queries to the server even if the cache has the data | ||
| // networkMode: 'always', // <-- This is not a solution, it just covers up some problem in our code, while disabling the biggest benefit of ReactQueries. Send queries to the server even if the cache has the data |
There was a problem hiding this comment.
For testing purposes, we need to have a reliable way to tell react-query to not cache anything. I don't think we have proven that we 100% understand all of the cases where react-query pulls data from its cache vs. directly from the API server. By being able to turn on a react-query configuration that forces only the freshest data from the API server, we can then isolate inconsistencies if there are any. Let's maintain the ability to test our theories about how react-query works in all edge cases before we assume that any caching or retrieval data bugs that we find are our fault. The react-query documentation describes a lot of conditions where it draws from the cache instead of pulling fresh data from the API server, and it will take time for us to experience all of those cases.
There was a problem hiding this comment.
With the queries that are implemented not in this checkin, networkMode: 'always' is not needed and it would be a mistake to casually use it, maybe wrap it in conditional test logic? ReactQuery works 100% of the time for me, and I've tested it extensively. It would be great if you showed me a case where it doesn't work.
In some cases I fell back to code from last week, leaving newer code that was not fully working in place, but commented out.
At this point every ReactQuery call works perfectly (without overriding the config values set in App.jsx) and I tested them many many times.
In TeamMemberList, I revived last week's working code, and left in place the code that uses apiCache. In TeamMemberList I now do a diff to show the differences from data straight out of ReactQuery (which has matched SQL every time that I have checked it manually), with the processed data that is stored in apiCache -- there currently is a discrepancy in apiDataCache. That diff should be useful in the future, and could be generalized.
I did not test or fix the Questionnaire drawer yet.
This is an incremental checkin, and I'll now work on getting the apiDataCache working as it exists. (Although it would be my strong preference to get it working by using ReactQuery's internal cache as the single cache for unaltered API results, and to generate the processed output through what is now apiDataCache, from data that is immediately pulled from ReactQuery's internal cache.)