fix(store): keep new contacts in the sorted index

[hamza221]

Fix #5681

Assisted-by: ClaudeCode:claude-opus-5

🤖 AI (if applicable)

• The content of this PR was partly or fully generated using AI

Summary by CodeRabbit

• Bug Fixes

  • Improved contact sorting to preserve value types and apply consistent favorite and secondary ordering.
  • Ensured contact additions, updates, bulk sorting, and address book replacements maintain the expected order.
  • Fixed sorting behavior when adding contacts to large existing contact lists.

• Tests

  • Added coverage for contact insertion in empty, beginning, middle, and end positions, including favorite contacts.

[hamza221]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 9094f2f0-1ec5-4669-a4cb-69e0ebd406ba

📥 Commits

Reviewing files that changed from the base of the PR and between 7d3b079 and 03c50d5.

📒 Files selected for processing (2)

• src/store/contacts.js
• tests/javascript/store/contactsMutations.test.js

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The contacts store now uses shared sorting helpers that preserve contact value types and prioritize favorites. Contact insertion, updates, replacement, and bulk sorting use the revised representation. New tests cover ordering, favorites, middle insertion, and a 300-contact regression case.

Changes

Contact sorting consistency

Layer / File(s)	Summary
Shared sorting entries and comparator src/store/contacts.js	The store adds sortedEntry and uses sortByFavoriteAndData for favorite priority and secondary ordering. Missing sort properties use empty-string fallbacks.
Mutation sorting paths and tests src/store/contacts.js, tests/javascript/store/contactsMutations.test.js	Contact insertion, updates, replacement, and bulk sorting use the shared comparator. Tests cover empty and populated lists, favorites, middle insertion, and the 300-contact regression case.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 03c50

New contacts are retained in the sorted contact index, allowing the creation form to open instead of showing “Contact not found.” The updated sorting behavior is covered across relevant insertion and ordering scenarios, with no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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 primary change: keeping newly created contacts in the sorted index.
Linked Issues check	✅ Passed	The changes address issue #5681 by revising contact sorting and insertion behavior so new contacts remain in the sorted index. Tests cover empty lists, insertion order, favorites, middle insertion, an…
Out of Scope Changes check	✅ Passed	All production and test changes are directly related to contact sorting and the new-contact regression described in issue #5681. No unrelated changes are present.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/sort/new-contact

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

tests/javascript/store/contactsMutations.test.js

Oops! Something went wrong! :(

ESLint: 10.10.0

ReferenceError: module is not defined in ES module scope
This file is being treated as an ES module because it has a '.js' file extension and '/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
at file:///tests/.eslintrc.js?mtime=1788785551929:5:1
at ModuleJob.run (node:internal/modules/esm/module_job:437:25)
at async node:internal/modules/esm/loader:639:26
at async dynamicImportConfig (/.eslint-tmp/node_modules/.pnpm/eslint@10.10.0_supports-color@8.1.1/node_modules/eslint/lib/config/config-loader.js:185:17)
at async loadConfigFile (/.eslint-tmp/node_modules/.pnpm/eslint@10.10.0_supports-color@8.1.1/node_modules/eslint/lib/config/config-loader.js:275:9)
at async ConfigLoader.calculateConfigArray (/.eslint-tmp/node_modules/.pnpm/eslint@10.10.0_supports-color@8.1.1/node_modules/eslint/lib/config/config-loader.js:594:23)
at async #calculateConfigArray (/.eslint-tmp/node_modules/.pnpm/eslint@10.10.0_supports-color@8.1.1/node_modules/eslint/lib/config/config-loader.js:369:19)
at async Promise.all (index 0)
at async findFiles (/.eslint-tmp/node_modules/.pnpm/eslint@10.10.0_supports-color@8.1.1/node_modules/eslint/lib/eslint/eslint-helpers.js:637:25)
at async ESLint.lintFiles (/.eslint-tmp/node_modules/.pnpm/eslint@10.10.0_supports-color@8.1.1/node_modules/eslint/lib/eslint/eslint.js:1025:21)

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

❌ Patch coverage is 57.89474% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/store/contacts.js	57.89%	6 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[hamza221]

/backport to stable8.8

[GVodyanov]

Claude found this:

⚠️ fixes the reported bug, introduces two new ones

The old insertion loop had its condition inverted (sortData(other, sortedContact) >= 0 → continue), so it inserted before the first contact that sorts earlier, and dropped the contact entirely when every existing contact sorted after it. Confirmed in the live store on the parent commit and fixed by the PR:

┌─────────────────────────────────────────────────────┬──────────────────────────┬───────────────────────┐
│                      scenario                       │          parent          │        PR 5708        │
├─────────────────────────────────────────────────────┼──────────────────────────┼───────────────────────┤
│ new contact sorts before all existing (issue #5681) │ dropped from the index   │ present at index 0 ✅ │
├─────────────────────────────────────────────────────┼──────────────────────────┼───────────────────────┤
│ new contact sorts in the middle                     │ Name, Alice, Zoe (wrong) │ Alice, Name, Zoe ✅   │
├─────────────────────────────────────────────────────┼──────────────────────────┼───────────────────────┤
│ contact with an empty name                          │ dropped from the index   │ present, but first ⚠️ │
├─────────────────────────────────────────────────────┼──────────────────────────┼───────────────────────┤
│ sort by First name, N:…;Mary,Jane;;; (vCard 4.0)    │ works                    │ TypeError ❌          │
└─────────────────────────────────────────────────────┴──────────────────────────┴───────────────────────┘

1. Crash when sorting by First/Last name. Removing .toString() from the index entry means value is no longer always a string: contact.firstName is n[1], which ICAL returns as an array when a vCard 4.0 N has multiple given names. sortData then takes the .toUnixTime() branch → TypeError: b.value.toUnixTime is not a function. It's reachable through import, which parses the raw file and keeps VERSION:4.0 before upload. End-to-end with sorting set to First name:

• parent: importing N:Wayne;Bruce,Bat;;; → accepted: 1, contact appears.
• PR 5708: importing N:Parker;Peter,Pete;;; → accepted: 0, denied: 1, contact absent from the list — but a PROPFIND shows it was created on the server. createVCard succeeds, then the addContact commit throws inside the promise and the .catch counts it as denied. Silent data/UI divergence until reload.

(Contacts fetched from the server come back as vCard 3.0, where N components stay strings — that's why normal browsing doesn't hit it.)

2. Nameless contacts move from the bottom to the top. The old sortByFavoriteAndName pushed empty values last (if (!a.value) return 1); sortByFavoriteAndData delegates to sortData, where '' localeCompares first. Visible with real data: sorting by First name puts all 14 account contacts (empty given name) above the named ones, where the parent commit put them below.

3. Undocumented behavior change: "Last modified" flips direction. rev now stays an ICAL.VCardTime, so sortData uses nameB - nameA → newest first, where the parent string-compared the ISO date → oldest first. A contact with no REV mixes a string into that comparison and sorts inconsistently against the typed ones.

Worth noting on the plus side: the PR removes a real pre-existing inconsistency — sortContacts built values without .toLowerCase() while setContacts/addContact applied it, so the index held mixed-case entries after a sort-order change.