[feature]: Inline constant creation in configurator#179
Open
deeonwuli wants to merge 24 commits intodevelopmentfrom
Open
[feature]: Inline constant creation in configurator#179deeonwuli wants to merge 24 commits intodevelopmentfrom
deeonwuli wants to merge 24 commits intodevelopmentfrom
Conversation
BundleMonNo change in files bundle size Groups updated (1)
Final result: ✅ View report in BundleMon website ➡️ |
xurxodev
requested changes
Apr 28, 2026
xurxodev
left a comment
There was a problem hiding this comment.
thanks @deeonwuli
Solid feature PR — the layering is right (codec → entity helpers → use case → repo → UI), the Monaco completion is encapsulated in its own module, and the design doc + tests for the JSON-path walker make the trickiest piece reviewable. Most comments are about regressions in the shared ConstantRepository.save contract, error visibility, and a few anys that should be typed.
1. Should fix
src/data/ConstantD2Repository.ts:15-50— chunking regression. The previoussavechunkedidsin groups of 100 and POSTed each chunk independently. The newbuildConstantPayloadcollapses everything into a single POST (existing items chunked only for the fetch step, but then concatenated and sent in one shot). For the inline-create path (one item) this is fine; forImportConstantTranslationsUseCaseand any future bulk import it's a regression. Either restore chunking on the POST, or assert thatsaveis only used for small batches and update the docstring of the repository method.src/data/ConstantD2Repository.ts— empty-array behaviour. With the rewrite,save([])now POSTs{ constants: [] }(before, the chunk loop made it a no-op). Add an earlyreturn Stats.empty()when bothexistingConstantsandnewConstantsare empty.src/domain/common/entities/User.ts:14— stray comment.// check if this is itlooks like a leftover TODO. Remove it (or replace with the resolved decision).src/webapp/.../monaco/registerInlineConstantCompletions.ts:107— silent fetch failure.options.fetchConstants(prefix).catch(() => [] as Constant[])swallows API errors. The user types intexts.X.code, the API fails, and they see "no constants" with no signal. Surface the failure: at minimumconsole.error, ideally a non-selectable completion item withdetail: i18n.t("Failed to load constants").src/webapp/.../monaco/registerInlineConstantCompletions.ts:25-35— typing. The function signature usesanyformonacoEditor, the model and thepositionareany, and the return objects areany. Use the types exported bymonaco-editor(editor.IStandaloneCodeEditor,languages.CompletionItem, etc.). The dead-code fallbackcommandId: registeredCommandId ?? commandId(the locally-generatedcommandIdis never registered withaddCommand, so the fallback would never fire a real command) should also be removed once typing forcesaddCommandto return astring.src/webapp/.../Editor.tsx:13,21—useState<any>().setEditorInstanceand therangeparameter onEditorApi.insertAtRangeuseany. Reuseeditor.IStandaloneCodeEditorandeditor.IRangeinstead of inventing a parallelMonacoRangeshape (also duplicated inConfigurator.tsxasPendingRange). Extract a smallmonaco/types.tsif you want to keep the import surface narrow.src/webapp/.../CreateConstantDialog.tsx:189— duplicate-code detection by regex on the error message./code/i.test(message) && /exist|duplicate|unique/i.test(message)will silently misclassify on locale changes or DHIS2 message tweaks. The right place is theerrorReports[].errorCodereturned intypeReports, whichStats.errorMessageflattens away. Two options: (a) enrichStatswith structured field/error data, or (b) haveCreateConstantUseCasethrow a typed error that includes the field. Acceptable for v1, but mark as a known fragility (TODO + follow-up issue).src/data/common/Dhis2DataStoreDataForm.ts:626— default value vs codec contract.defaultDataStoreConfig.prefix = ""while the codec types itoptional(string). Either align the default toundefined(and let consumers?? ""), or change the codec to a non-optionalstringwith a default. Right now the docs say "absent ⇒ no filter" butdefaultDataStoreConfigmaterialises an empty string.
2. Recommendations non blocking
extractRootPrefix.tshas no unit tests. Add cases for: prefix absent, prefix as a non-string, escaped characters in the value, prefix in the middle of mid-typed (invalid) JSON. The regex fallback's.replace(/\\(.)/g, "$1")un-escapes too aggressively (\n→n); fine for ASCII prefixes but worth a comment narrowing the supported character set.jsonPathAtCursor.ts— extra walker tests. The 7 existing cases cover the happy path. Hand-rolled JSON traversal benefits from tests for: cursor inside a string with escaped quotes ("a\"b"),codeinside an array element undertexts, very deeply nested objects, and trailing comma / mid-typed malformed JSON.
The previous refactor collapsed all constants into a single POST, regressing ImportConstantTranslationsUseCase. Re-chunk in groups of 100 on the POST step, return Stats.empty() up front when there is nothing to save, and surface structured errorReports through Stats so callers can react to specific field-level errors instead of regex-matching the flattened message.
…Case Replace the regex-on-error-message duplicate-code detection in the configurator dialog with structured errorReports. The use case now throws ConstantSaveError carrying the DHIS2 errorReports, and the dialog maps each report to its corresponding form field via errorProperty rather than parsing locale-sensitive message strings.
The codec defines `prefix` as `optional(string)`, but the in-memory default config materialised it as the empty string and the Right branch substituted "" when the field was absent. Keep the field optional in the in-memory shape and pass the decoded value through unchanged so "absent ⇒ no filter" holds end-to-end.
Add a small monaco/types.ts with structural interfaces matching the subset of monaco-editor we use (CodeEditor, TextModel, Position, IRange, CompletionItem). Replace the `any` types and the duplicated range shape in Editor.tsx, Configurator.tsx, and the completion provider with these. Drop the dead-code commandId fallback (the local id was never registered with addCommand). When the constants fetch fails, log the error and emit a non-selectable completion item labelled "Failed to load constants" instead of silently showing an empty list.
…lker Add a spec for extractRootPrefix covering: prefix absent or empty, non-string prefix values, escaped quotes in the value, and the regex fallback used while the surrounding JSON is mid-typed. Extend the jsonPathAtCursor spec with escaped quotes inside string values, deeply nested objects, and trailing-comma / unbalanced-brace paths the user produces while editing. Document the walker's current behaviour around array ancestors so a future change can revisit it intentionally.
… constant creation
- Add Monaco autocomplete for `texts.*.code` fields with a "Create new constant" entry. - Introduce `CreateConstantDialog` for creating new constants inline. - Implement auto-derivation for `shortName` and `code` based on user input. - Ensure constant persistence via DHIS2 API with error handling for duplicates and validation. - Gate the creation feature based on user permissions (`F_CONSTANT_ADD`). - Maintain functionality in large-file mode by disabling unnecessary suggestions. - Update specifications and tasks to reflect new features and requirements.
…://github.com/EyeSeeTea/d2-autogen-forms into feat/inline-constant-creation-in-configurator
xurxodev
approved these changes
May 5, 2026
xurxodev
left a comment
There was a problem hiding this comment.
thanks @deeonwuli
- Recommended non-blocking
The IA agents configuration is duplicated, but I think this is a topic to treat in the coding dojo meeting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 References
📝 Implementation
"code": "..."value (anytexts.*.codereference), the suggestion widget shows a Create new constant entry. Selecting it opens a dialog to define the constant (name, short name, code, description) and saves it to DHIS2.openspec/changes/inline-constant-creation/(proposal.md,design.md,tasks.md+ delta spec) and lives long-term asopenspec/specs/inline-constant-creation/spec.md.CreateConstantDialogso users can upload different translations.🎨 Screenshots
Screen.Recording.2026-04-29.at.14.55.41.mov
🔥 Notes to the tester
Test against
https://dev.eyeseetea.com/who-dev-41/with a user that hasF_CONSTANT_ADD(orALL).texts.*.codevalue → widget shows a singleCreate new constantentry. Selecting it opens the dialog. Save creates the constant in DHIS2 and inserts its code into the JSON at the cursor.F_CONSTANT_ADD. The create entry should appear marked deprecated (struck through) with detailRequires F_CONSTANT_ADD authority; selecting it should be a no-op.shortNamelonger than 50 characters should also block.codethat already exists in DHIS2. The dialog should surface a per-field error oncode.#869ckzqxc