feat(ui): allow user to customize PR title - #6123
Conversation
There was a problem hiding this comment.
Verdict: request changes
The PR correctly moves proposal template rendering to the backend so the UI can pre-fill editable defaults, which is a solid UX improvement. However, the prior review’s blocking concern is still unaddressed: Propose unconditionally renders templates and hard-fails on rendering errors even when the caller supplied a custom title and body that would discard the rendered result, regressing from the previous behavior.
internal/coss/storage/environments/git/store.go
- major (L146): Propose unconditionally calls renderProposalDefaults and returns its error before checking opts.Title/opts.Body. Previously, supplying custom values skipped template execution entirely, so a broken or unreadable repository-level template did not block proposals. Now it does. Only render defaults when the caller hasn’t provided a value, or make template failure non-fatal when overrides are present.
🤖 Automated review by the Flipt PR review agent.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #6123 +/- ##
==========================================
- Coverage 62.28% 62.26% -0.02%
==========================================
Files 143 143
Lines 14729 14733 +4
==========================================
Hits 9174 9174
- Misses 4809 4813 +4
Partials 746 746
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
markphelps
left a comment
There was a problem hiding this comment.
great idea @jgaul3 ! I think we need to reconcile this (and document) how proposal templates fit in though
|
|
||
| const [proposeEnvironment] = useProposeEnvironmentMutation(); | ||
|
|
||
| const defaultTitle = `Flipt: Update features ${ |
There was a problem hiding this comment.
how does this work with the (not well documented at all) feature of templates?
Line 37 in d2f4345
There was a problem hiding this comment.
Ah, that is just a flat oversight on my part, I thought the default we were working with was the one on the backend. How about I put in a new api endpoint to get the proposal title value?
There was a problem hiding this comment.
I'm inclined to put the proposal title value on ListBranchedEnvironmentChangesResponse but could do it separately as well.
There was a problem hiding this comment.
There was a problem hiding this comment.
@markphelps Thanks for the feedback! The GetInfo API is much easier to use. A couple tradeoffs, it currently just shows the template. We could get the values to populate it in the frontend but the code to do the populating got pretty unpleasant. Or we could populate those branch / environment values in the backend, but I'm not sure I could see a way to do so via the GetInfo endpoint... Please let me know what you think of this crack at a solution and I'll adjust as needed.
There was a problem hiding this comment.
yeah i see what you mean. ill add an API to get the rendered template from the backend so it can just be used by the frontend directly. will put up a pr today that we can rebase this one onto
There was a problem hiding this comment.
So I just pushed a follow-up commit onto your branch (kept your commit as-is) that renders the proposal title/body on the backend from the hydrated templates and returns them via ListBranchedEnvironmentChanges, so the modal pre-fills the real, rendered default that the user can override.
The UI already called this to get the commits so no extra roundtrips
Signed-off-by: Jon Gaul <jgaul3@gmail.com>
Render the merge-proposal title and body on the backend from the hydrated templates (built-in defaults overlaid with server- and repository-level overrides) and return them via ListBranchedEnvironmentChanges so the UI can pre-fill the form. Users may override the rendered defaults; caller-supplied values continue to be used verbatim. This replaces exposing the raw, unrendered server template through /meta/info, which could not reflect real branch refs or repo-level overrides and led to unrendered template syntax appearing in proposal titles. Also fixes a pre-existing Input.tsx ref type error surfaced by the UI build. Signed-off-by: Mark Phelps <209477+markphelps@users.noreply.github.com>
Signed-off-by: Mark Phelps <209477+markphelps@users.noreply.github.com>
|
@erka do you mind reviewing since I contributed to this branch? |
Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
erka
left a comment
There was a problem hiding this comment.
nice one.
I bumped buf to 1.71 to match mise.
Description
Might be nice to let the user change the title of the resulting PR when they spin off their changes. Fully renders the template on the backend with proper branch names.
Screenshot