[test]: verify needsSaved propagates from independent collection to p…#8258
[test]: verify needsSaved propagates from independent collection to p…#8258rijulpoudel wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthroughAdded tests in ChangesIndependent Collection needsSaved Propagation Test
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…d and persist after save
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts (1)
501-509: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a companion assertion for
remove()propagation.Coverage here is add-only. A symmetric case (removing an item from the independent collection also triggers parent
needsSaved) would round out regression coverage for#8241with minimal extra setup, reusing the same fixtures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts` around lines 501 - 509, Add a symmetric regression check in resourceApi.test.ts alongside the existing independentCollection add coverage: reuse the same fixtures to verify that calling remove() on an independent collection also propagates saverequired so parentResource.needsSaved becomes true while the removed resource itself is not marked needsSaved. Use the existing independentCollection, parentResource, and Resource setup to keep the new assertion minimal and consistent with the current add/remove behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts`:
- Around line 501-509: Add a symmetric regression check in resourceApi.test.ts
alongside the existing independentCollection add coverage: reuse the same
fixtures to verify that calling remove() on an independent collection also
propagates saverequired so parentResource.needsSaved becomes true while the
removed resource itself is not marked needsSaved. Use the existing
independentCollection, parentResource, and Resource setup to keep the new
assertion minimal and consistent with the current add/remove behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e18c4f2-2381-45b5-87c7-27afc8e5ef4f
📒 Files selected for processing (1)
specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts
[test]: verify independent resource field changes propagate needsSave…
verify independent subview sub-record changes propagate needsSaved an…
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts (1)
517-589: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTests look solid — well-structured and cover the issue
#8241objectives.The
setupParentWithIndependentCollectionhelper nicely reduces duplication, and both test cases verify propagation and persistence as intended.One minor optional note: the
as anycasts forstoreIndependent(lines 537, and also 496 in the first suite) are well-documented, but ifstoreIndependentis used more broadly in tests, adding it to the public TypeScript types would eliminate the need for these casts long-term.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts` around lines 517 - 589, The tests are relying on `as any` casts to call `storeIndependent`, which hides a typing gap in the public API. Update the TypeScript types for `storeIndependent` (and any related resource/collection interfaces used in `setupParentWithIndependentCollection` and the earlier suite) so the method is available without casts, then remove the `as any` usages in these tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts`:
- Around line 517-589: The tests are relying on `as any` casts to call
`storeIndependent`, which hides a typing gap in the public API. Update the
TypeScript types for `storeIndependent` (and any related resource/collection
interfaces used in `setupParentWithIndependentCollection` and the earlier suite)
so the method is available without casts, then remove the `as any` usages in
these tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2df6d622-48e6-471c-9e6b-91cde013db35
📒 Files selected for processing (1)
specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts
Fixes #8241
Summary by CodeRabbit