List what an import holds back and leave out a group that would stand empty

[SirLouen]

Closes #242

What

When a file moves a field into another group and the move is left unticked, the import now lists that field under what it left alone, together with every field inside it. A new group the file declares only for such fields is no longer created, and it is listed there too. A new group that also gains a fresh field is still created, holding the fresh field alone. A kind change nobody confirmed now lists the fields inside the changed field as well.

Why

The import kept the field where it was, as promised, but it created the new group empty and said nothing about the field it held back. The admin was left with an empty group and no word about the move they had declined.

Testing Instructions

1. In the admin, add a field group "Article details" for posts with a text field "Subtitle", then export the definitions.
2. In the file, move "Subtitle" out of "Article details" into a new group with the key article-facts and the title "Article facts", also for posts.
3. Import the file and leave the move unticked. No "Article facts" group appears, "Subtitle" stays in "Article details", and the list of what was left alone shows "Remove Subtitle", "Add Article facts" and "Add Subtitle". Before this change it showed only "Remove Subtitle".
4. Add a second, new field to "Article facts" in the file and import it again, still unticked. The group now stands with the new field alone, and "Add Subtitle" is still listed as left alone.
5. Run make cover and make lint.

Summary by CodeRabbit

• Bug Fixes
  • Imports now leave declined field moves unchanged and do not create groups used only by those fields.
  • Import reports now list all changes left undone, including nested fields and unconfirmed replacements.
  • Groups are included in the requested order only when they are stored.
• Documentation
  • Clarified how declined field moves and skipped changes are handled during imports.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: gopherium/gophenberg/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e75fe69f-aab8-4c83-8f0c-d26220b234eb

📥 Commits

Reviewing files that changed from the base of the PR and between 4501aaf and 9a51ed4.

📒 Files selected for processing (7)

• docs/src/content/docs/guides/fields.md
• internal/definitions/apply.go
• internal/definitions/apply_held_test.go
• internal/definitions/applyfields.go
• test/features/features/field-groups.feature
• test/features/steps_definitions_test.go
• test/features/world_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Import application now reports declined fields and their planned descendant changes as skipped. It also skips creating a new group when every field in that group is held back. Tests and documentation cover the resulting import outcome.

Changes

Held-back import changes

Layer / File(s)	Summary
Record skipped fields and omit empty groups internal/definitions/apply.go, internal/definitions/applyfields.go, internal/definitions/apply_held_test.go	Declined fields and replacement changes, including descendant fields, are recorded as skipped. A new group is skipped when all its fields are held back. Unit tests cover fully held-back groups, groups with fresh fields, moved sections, and unconfirmed replacements.
Verify and document skipped import changes test/features/features/field-groups.feature, test/features/steps_definitions_test.go, test/features/world_test.go, docs/src/content/docs/guides/fields.md	Feature-test steps retain the import response and assert which groups and fields were left undone. The feature scenario and guide describe the declined field move and its reported outcome.

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

Merge Risk: ⚪ Minimal · up to 9a51e

The held-back import behavior has no identified issue blocking merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. (2 skipped: …	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 summarizes the main changes: report held-back import changes and omit a group that would contain no stored fields.
Linked Issues check	✅ Passed	The implementation meets #242. groups skips a newly declared group when all its declared fields are declined, and it records the group create as skipped. oneField records declined fields, and `lef…
Out of Scope Changes check	✅ Passed	The changes stay within #242 and the stated import-reporting behavior. The implementation changes group creation and skipped-change reporting. The unit tests and feature test verify those changes. The…

Full details: Docstring Coverage

Explanation

Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[greptile-apps]

Confidence Score: 5/5

Safe to merge: the changed import behavior passed both database-backed and HTTP-level checks.

What we checked:

• Ran an identical database-backed import probe on the base revision and the current revision using the Import behavior validation script.
• The base revision failed all three checked cases in that probe.
• The current revision passed all three cases against disposable PostgreSQL-backed registries.
• Exercised the Field Groups feature scenario through the HTTP definitions import endpoint; the destination group is absent, the source field remains, and the response reports the skipped group and field.
• Compared the earlier probe behavior with the authored executable after changes; before: greptile-base exited 1 reproducing the missing behaviors, after: HEAD exited 0 for the same generated probe.

Summary

This change fixes imports that leave destructive field moves or replacements unconfirmed: the existing field subtree remains intact, an otherwise-empty destination group is omitted, and the import result names every skipped field change. A destination group with an independent new field is still created. Database-backed import checks and the HTTP definitions-import flow confirm the behavior.

_{Reviews (1) · Last reviewed commit: "docs(guides): say what an import does wi..."}