TT-7386 Assign section from sheet#360
Conversation
gtryus
changed the title
There was a problem hiding this comment.
Pull request overview
This PR improves correctness of “assign section from sheet” behavior by ensuring the sheet view is rebuilt from the most up-to-date Orbit Memory cache records and by better sequencing remote-side side effects when saving permission schemes.
Changes:
- Added a unit test to validate that passage assignees are derived from
organizationschemesteprecords tied to a section’sorganizationScheme. - Updated
ScriptureTable’srefreshSheetto re-query sections and organization scheme steps directly frommemory.cachebefore callinggetSheet. - Updated
AssignSectionto wait for the Orbit remote request queue to drain after saving a permission scheme and before running the assign call.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/renderer/src/components/Sheet/ScriptureTable.tsx | Rebuilds the sheet using freshly queried sections and organization scheme steps from the Orbit Memory cache. |
| src/renderer/src/components/AssignSection.tsx | Adds a remote-queue wait after saving scheme changes to reduce race conditions before assigning sections. |
| src/renderer/src/tests/getSheet.test.ts | Adds coverage for selecting a passage assignee from an organization scheme step linked to the section’s scheme. |
| setBusy(true); | ||
| try { | ||
| const schemeId = await handleAdd(); | ||
| await waitForRemoteQueue('permission scheme save'); |
There was a problem hiding this comment.
perhaps waitForRemoteId would be better here?
There was a problem hiding this comment.
We have several tables to wait on. All the organiztionschemasteps, as well as the organizationschemes (which finishes first.) One of the last things to get updated is the sections table.
There was a problem hiding this comment.
I can completely leave out this change to AssignSection and the code still solves the issue. Normally Assign section is waiting until all the changes are written before it closes so we can simplify the PR by leaving this out.
| ) as SectionD[]; | ||
| const freshSchemeSteps = memory.cache.query((q) => | ||
| q.findRecords('organizationschemestep') | ||
| ) as OrganizationSchemeStepD[]; |
There was a problem hiding this comment.
If we can't rely on useOrbitData we're in trouble. sections is in the callback refresh list. Why do we need to get from memory????? This feels like extra code that we shouldn't need.
There was a problem hiding this comment.
when I removed this code and just relied on what was there (a dependency which includes sections and and organizationshemestepss, the sheet isn't updated like it should be. :-(
gtryus
force-pushed
the
TT-7386-sheet-assign-refresh
branch
from
532b36e to
88f84e1
Compare
ScriptureTablecomponent to refresh sections and organization scheme steps from the memory cache, ensuring the latest data is used when rendering the sheet.AssignSectioncomponent to handle asynchronous operations more effectively.