[SirLouen]

Closes #622

What

The import preview now refuses a type or field key with spaces around it, the way it already refuses such a group key.

Why

The preview read these keys as written and the import trimmed them, so a file could pass the preview and then fail partway. A spaced field key cost the field groups the admin had ticked for removal.

Testing Instructions

1. Export definitions, put spaces around a field key in the file, then import it. The preview shows the key error and nothing changes.
2. Import the file as exported. The preview finds nothing to change.

Summary by CodeRabbit

• Bug Fixes
  • Imports now reject type, group, and field keys with surrounding spaces instead of silently trimming them. Invalid keys are reported before import changes are applied.
• Documentation
  • Updated the field import guide to explain how keys with surrounding spaces are handled.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: gopherium/gophenberg
• Branch: fix/622
• File: .coderabbit.yaml
• Commit: f3d98b7
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: a22f9c9a-b5b6-4596-93ae-255d522b8723

Commits

Reviewing files that changed from the base of the PR and between 7e00565 and f3d98b7.

Files selected for processing: 8

• docs/src/content/docs/guides/fields.md
• internal/definitions/derived_route_word_internal_test.go
• internal/definitions/padded_key_test.go
• internal/definitions/pagination_internal_test.go
• internal/definitions/route_word_order_internal_test.go
• internal/definitions/validate.go
• test/features/features/field-groups.feature
• test/features/steps_definitions_test.go

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

Walkthrough

Definition imports now reject type and field keys with surrounding spaces instead of normalizing them. Tests cover comparison and application behavior, including preservation of a confirmed group when import validation fails. Feature scenarios and documentation describe the rejection rule.

Changes: Definition key validation

Layer / File(s)	Summary
Validation and import tests internal/definitions/validate.go, internal/definitions/padded_key_test.go, internal/definitions/*_internal_test.go	Type keys remain as written during validation, and field keys with surrounding spaces are rejected. Tests cover comparison, plugin-declared types, import application, and confirmed-group preservation. Related route-word tests now use unpadded keys.
Feature coverage and documentation test/features/features/field-groups.feature, test/features/steps_definitions_test.go, docs/src/content/docs/guides/fields.md	Feature scenarios check that planning refuses padded type and field keys. The guide states that imports refuse padded type, group, and field keys before making changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• gopherium/gophenberg#592: Both changes add pre-import validation and protect confirmed groups when validation fails.
• gopherium/gophenberg#491: Both changes refuse invalid definitions before confirmed groups are deleted.
• gopherium/gophenberg#617: Both changes address inconsistency between import planning and application.

Suggested reviewers: codescene-access

Merge Risk: Minimal · up to f3d98

The key-validation changes, accompanying tests, and documentation are consistent with rejecting padded keys before import changes. No merge-blocking issue is established; merge after normal checks pass.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title says the change refuses a key with spaces around it at the preview. The changeset does this. validate.go now rejects field keys that differ from their trimmed form, and typeFrom no longer tr…
Linked Issues check		Issue #622 requires preview rejection of type and field keys with surrounding spaces, with no partial import. typeFrom now preserves the declared type key, and fieldStands rejects a field key that…
Out of Scope Changes check		The changed validation code, unit tests, feature tests, and documentation directly support issue #622. The adjusted route-word and label test cases remove or update cases that now conflict with the re…

Full details: Title check

Explanation

The title says the change refuses a key with spaces around it at the preview. The changeset does this. validate.go now rejects field keys that differ from their trimmed form, and typeFrom no longer trims type keys, so padded keys are refused. Feature scenarios and Go tests cover refusal of padded type and field keys with type_key_malformed and field_key_malformed. The title names the primary change, and the summary's focus on preview rejection matches the linked issue. The title is specific, under 75 characters, and uses the conventional prefix. Minor gap: it does not mention that group keys were already handled or that existing tests were adjusted.

Full details: Linked Issues check

Explanation

Issue #622 requires preview rejection of type and field keys with surrounding spaces, with no partial import. typeFrom now preserves the declared type key, and fieldStands rejects a field key that differs from its trimmed form. The added tests cover new, existing, duplicate, nested, default, and plugin-declared types and fields. The Apply test confirms that a confirmed group remains stored when validation fails. Feature tests cover type_key_malformed and field_key_malformed.

Full details: Out of Scope Changes check

Explanation

The changed validation code, unit tests, feature tests, and documentation directly support issue #622. The adjusted route-word and label test cases remove or update cases that now conflict with the required padded-key rejection. No unrelated production behavior is shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 10 functions across 6 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• 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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[codescene-access]

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[gitar-bot]

Code Review ✅ Approved

🟡 Medium risk · The import now rejects padded type and field keys, which could refuse such files and block intended definition updates.

Import preview now refuses type and field keys with spaces around them, matching the existing group key check, so a spaced key fails at preview instead of partway through the import. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] Safe to merge. The exercised malformed imports were rejected before confirmed groups could be removed, while a valid import still completed its requested deletion.

Summary

The import preview now rejects padded type and field keys before any changes are written.

• Definition imports refuse type and field keys with surrounding spaces.

No actionable issues were found. The change is safe to merge based on the reviewed code and executed memory-backed API checks. PostgreSQL-backed integration tests were not run.

_{Reviews (1) · Last reviewed commit: "docs(guides): say an import reads keys a..." · Reviewed by Greptile}