Repository navigation
fix(profiles): preserve Space associations in single exports - #1385
Belligerently wants to merge 3 commits into
Conversation
Signed-off-by: Belligerently <zachary@aflalo.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
| Layer / File(s) | Summary |
|---|---|
Include and verify Space metadata Thaw/Settings/Models/ProfileManager.swift, ThawTests/Settings/Models/ProfileManagerCRUDTests.swift, CHANGELOG.md |
exportProfile(id:to:) adds the associated Space key and name to the export entry. The regression test checks the association after import and manager reload. The changelog notes the fix. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers: diazdesandi
Merge Risk: 
Minimal · previous review b1e38
-
The change correctly preserves Space associations through single-profile export and reload.
Pre-merge checks: 
5
-
Full details: Description check
-
Explanation
The description follows the repository template. It includes the summary, linked issue, PR type, area, breaking-change status, new behavior, test commands, known limitations, and other information. It also clearly reports incomplete validation items.
Full details: Linked Issues check
-
Explanation
Issue
#1384is the only directly linked issue. Its coding requirement is that single-profile export keeps associatedSpaceKey and associatedSpaceName, as Export All does, so that import on the same Mac restores the Space binding and label. The whole-PR summary states that ProfileManager.exportProfile(id:to:) now adds both Space values to the exported entry, with nil values when no matching profile metadata exists. The PR description states the same behavior. The issue reports that import already supports these fields. The test ThawTests/Settings/Models/ProfileManagerCRUDTests.swift adds exportImportPreservesSpaceAssociation. It exports and imports a Space-bound profile in a temporary directory, reloads the manager, and checks the key and label. This covers the required regression test. Limit: the repository diff tool could not read the changed blobs, and the head file read was truncated before exportProfile. The verdict relies on the whole-PR summary, which is consistent across the PR description, the issue, and the previous assessment. The author reports the 34 ProfileManagerCRUDTests passed, but this review did not run them.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR summary lists three changed files. ProfileManager.swift changes only the single-profile export entry by adding the two Space fields named in
#1384. ProfileManagerCRUDTests.swift adds only the export/import/reload regression test for those fields. CHANGELOG.md adds a Fixed entry for the same behavior. No public API declaration, dependency, or other production behavior change appears in the summary. The PR description reports no new state or responsibilities. Limit: the direct diff could not be read, so the assessment depends on the summary. This does not show any unrelated change.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
Labels: All convention checks passed. ✅ |
Signed-off-by: Belligerently <zachary@aflalo.com>
diazdesandi
left a comment
There was a problem hiding this comment.
Resolve conflicts so I can merge
Signed-off-by: Belligerently <zachary@aflalo.com>
8325f69 to
e071e51
Compare
|
@diazdesandi The CHANGELOG.md conflict is resolved against legacy/thaw-2, preserving the release notes and the export fix under Unreleased. All three PR commits now show Verified signatures; the signing update did not change file contents. Could you take another look and update your review when you have a chance? The workflows are also awaiting maintainer approval. Thanks! |
Summary
Preserve Space bindings and labels in single-profile exports, matching Export All. Add a regression test and changelog entry. This small fix repairs ProfileManager’s existing export responsibility and adds no state or responsibilities.
Linked issue (required)
Closes: #1384
PR Type
Area
Does this PR introduce a breaking change?
What is the new behavior?
Single-profile export includes associatedSpaceKey and associatedSpaceName. The test exports and imports a Space-bound profile in a temporary directory, reloads the manager, and verifies its association and label persisted.
PR Checklist
Test commands run on macOS 27.2 (26B118), Xcode 27.1:
Build passed; all 34 profile CRUD tests passed. The xctestrun copy sets XCODE_RUNNING_FOR_PREVIEWS=1 in the test environment to skip normal app startup and live menu-bar management.
SwiftFormat 0.64.1: swiftformat . completed; unrelated formatting was restored.
SwiftLint 0.65.1: strict lint of ProfileManager.swift passed with zero violations. Repository-wide strict lint with DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer reports 11 legacy_swiftui_aspect_ratio violations in unchanged files. CI uses 0.63.3. No suppressions added.
Git diff --check passed.
Known limitations / follow-ups
No interactive app check or macOS 26 verification. Automated tests ran on macOS 27.2. Repository-wide strict lint with the CI version remains outstanding; local results are above. SonarCloud coverage and quality gate are not claimed.
Other information
AI-assisted change reviewed for scope and existing persistence semantics. No external code or dependencies added.
Summary by CodeRabbit