Skip to content

fix(admin): let the add-app dialog accept a typed branch - #378

Merged
tanmoysrt merged 1 commit into
feat/setup-branch-comboboxfrom
fix/add-app-dialog-custom-branch
Aug 27, 2026
Merged

fix(admin): let the add-app dialog accept a typed branch#378
tanmoysrt merged 1 commit into
feat/setup-branch-comboboxfrom
fix/add-app-dialog-custom-branch

Conversation

@mihir-kandoi

@mihir-kandoi mihir-kandoi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Stacked on #377.

The add-app dialog's manual-URL branch picker passes allowCustomValue, which doesn't exist in frappe-ui 1.0.0-beta.34 — it lands on the <input> as an inert DOM attribute, so the promised "Type one to use it" flow silently never commits. This extracts #377's pick-or-type options into a shared branchComboboxOptions helper, points both the wizard and the dialog at it, and switches the dialog's manual picker to the same button-trigger custom-row pattern. The connected-GitHub picker stays list-only, as before.

Typed branches still go through resolveApp, so an invalid name can't be submitted. Unit tests cover the helper via the existing node --test harness; npm run build passes.

image

@mihir-kandoi
mihir-kandoi force-pushed the fix/add-app-dialog-custom-branch branch from 48d1fbf to d08ab84 Compare August 25, 2026 13:52
@mihir-kandoi
mihir-kandoi force-pushed the fix/add-app-dialog-custom-branch branch from d08ab84 to 70a8586 Compare August 25, 2026 14:25
@tanmoysrt
tanmoysrt force-pushed the fix/add-app-dialog-custom-branch branch from 70a8586 to 2d267e8 Compare August 26, 2026 08:32
@mihir-kandoi
mihir-kandoi force-pushed the fix/add-app-dialog-custom-branch branch 2 times, most recently from 303b2f7 to 4533363 Compare August 26, 2026 09:46
@mihir-kandoi
mihir-kandoi force-pushed the fix/add-app-dialog-custom-branch branch 2 times, most recently from 982bdd2 to 3919e92 Compare August 26, 2026 10:01
@tanmoysrt
tanmoysrt force-pushed the fix/add-app-dialog-custom-branch branch from 3919e92 to 37187e2 Compare August 26, 2026 10:05
@tanmoysrt
tanmoysrt force-pushed the fix/add-app-dialog-custom-branch branch from 37187e2 to 92a5035 Compare August 27, 2026 10:41
The manual-URL branch picker passed allowCustomValue, which does not
exist in frappe-ui 1.0.0-beta.34 - it fell through to the input as an
inert DOM attribute, so a typed branch never committed even though the
empty state promised it would. Use the shared branchComboboxOptions
typed-branch row instead, in button trigger mode like the wizard.

The connected-GitHub picker keeps its list-only options: its branches
come from the authenticated listing, and it never offered free text.
@tanmoysrt
tanmoysrt force-pushed the fix/add-app-dialog-custom-branch branch from 92a5035 to 604498f Compare August 27, 2026 12:16
@tanmoysrt

Copy link
Copy Markdown
Member

@greptile review

@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The PR appears safe to merge, with the non-blocking request to cover the dialog’s typed-branch interaction.

The implementation mirrors an existing branch-picker pattern, but helper-only tests do not protect the newly changed component wiring.

Files Needing Attention: admin/frontend/dashboard/src/components/apps/AddAppFromGithubDialog.vue

Prompt To Fix All With AI
### Issue 1
admin/frontend/dashboard/src/components/apps/AddAppFromGithubDialog.vue:216
**Untested dialog integration**

Add a regression test that exercises typing and selecting a custom branch through this dialog; the existing helper-only tests do not cover the changed `trigger`, slot, and `v-model` integration, so that user flow can regress while the tests remain green.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(admin): let the add-app dialog accep..." | Re-trigger Greptile

@tanmoysrt
tanmoysrt merged commit 7199e49 into develop Aug 27, 2026
6 checks passed
@tanmoysrt
tanmoysrt deleted the fix/add-app-dialog-custom-branch branch August 27, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants