Skip to content

[test]: Clone Loan — verify preparations are not carried over#8290

Open
rijulpoudel wants to merge 1 commit into
mainfrom
issue-8289
Open

[test]: Clone Loan — verify preparations are not carried over#8290
rijulpoudel wants to merge 1 commit into
mainfrom
issue-8289

Conversation

@rijulpoudel

@rijulpoudel rijulpoudel commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Fixes #8289

Summary by CodeRabbit

  • Tests
    • Added coverage to ensure cloned loan records start with a clean state and do not carry over dependent preparation data.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Warning

One or more dependencies are approaching or past End-of-Life.
Please plan upgrades accordingly.

STATUS=WARNING
NODE_VERSION=20
NODE_CYCLE=20
EOL_DATE=2026-04-30
DAYS_REMAINING=-70

--- Node.js ---
Version: 20
EOL: 2026-04-30
Status: WARNING

STATUS=OK
PYTHON_VERSION=3.12
PYTHON_CYCLE=3.12
EOL_DATE=2028-10-31
DAYS_REMAINING=845

--- Python ---
Version: 3.12
EOL: 2028-10-31
Status: OK

STATUS=WARNING
DJANGO_VERSION=4.2
DJANGO_CYCLE=4.2
EOL_DATE=2026-04-07
DAYS_REMAINING=-93

--- Django ---
Version: 4.2
EOL: 2026-04-07
Status: WARNING


@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

A new Jest test case was added to verify that cloning a Loan resource does not carry over dependent loanPreparations. The test mocks console.warn, clones a Loan instance, and asserts the cloned resource's loanPreparations collection resolves as empty.

Changes

Loan Clone Test

Layer / File(s) Summary
Clone loan preparations test
specifyweb/frontend/js_src/lib/components/DataModel/__tests__/domain.test.ts
Adds a test verifying loan.clone(true) produces a cloned resource whose loanPreparations collection is empty, mocking console.warn during the assertion.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a test for loan cloning that verifies preparations are not carried over.
Linked Issues check ✅ Passed The PR directly addresses issue #8289 by adding a test case that verifies loan preparations are not carried over when cloning, which matches the linked issue requirement.
Out of Scope Changes check ✅ Passed All changes are scoped to adding a test case for loan cloning behavior as specified in the linked issue; no unrelated modifications are present.
Automatic Tests ✅ Passed PR adds an automatic test in domain.test.ts covering loan cloning behavior, so the check is satisfied.
Testing Instructions ✅ Passed The new regression test clearly targets Loan.clone and verifies loanPreparations are dropped, matching the affected component and issue scope.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-8289

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
specifyweb/frontend/js_src/lib/components/DataModel/__tests__/domain.test.ts (1)

162-162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a comment explaining the console.warn mock.

The sibling test at line 186-188 includes a comment explaining why console.warn is mocked. Adding a similar comment here would help future readers understand why the mock is needed during loan cloning.

💡 Suggested addition
   jest.spyOn(console, 'warn').mockImplementation();
+  // Suppress warnings from inline data handling during clone
   const cloned = await loan.clone(true);
🤖 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__/domain.test.ts`
at line 162, The `domain.test.ts` loan cloning test mocks `console.warn` without
explaining why, unlike the sibling test, so add a brief comment near the
`jest.spyOn(console, 'warn').mockImplementation()` call in the loan cloning test
to clarify the purpose of the mock and keep the test intent obvious for future
readers.
🤖 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__/domain.test.ts`:
- Line 162: The `domain.test.ts` loan cloning test mocks `console.warn` without
explaining why, unlike the sibling test, so add a brief comment near the
`jest.spyOn(console, 'warn').mockImplementation()` call in the loan cloning test
to clarify the purpose of the mock and keep the test intent obvious for future
readers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d01859db-739c-4914-9445-bd7005ef82ca

📥 Commits

Reviewing files that changed from the base of the PR and between e5e7ac2 and 4b1ad5d.

📒 Files selected for processing (1)
  • specifyweb/frontend/js_src/lib/components/DataModel/__tests__/domain.test.ts

@rijulpoudel
rijulpoudel requested a review from CarolineDenis July 9, 2026 20:26
@rijulpoudel rijulpoudel added this to the 7.12.1 milestone Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 📋Back Log

Development

Successfully merging this pull request may close these issues.

[test]: clone loan - verify preparations are not carried over

1 participant