feat(web): merge merge requests asynchronously with htmx and live state updates
The merge form posts through htmx for every mode and swaps the MR state badge and action area in place; refusals and failures render a status fragment instead of a full error page. Non-htmx posts keep the redirect. Other open MR tabs receive the merged state through the live stream, and the checks label observer watches the stable MR bar so it survives swaps.
- commit
- 5fea81ff21bc05f0d324773ff9d33ac9572363be
- author
- brice · · 2026-10-22 00:54 UTC
- parent
- 53e25a3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +1475 -1067
M internal/adapters/web/fragments.go +15 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "bytes" | |
| 7 | 7 | "context" | |
| 8 | 8 | "errors" | |
| 9 | + | "net/http" | |
| 9 | 10 | "strconv" | |
| 10 | 11 | "strings" | |
| 11 | 12 | "sync" | |
| @@ -192,6 +193,20 @@ func (l *liveStream) repoPage(ctx context.Context, we ports.WatchEvent) ([]fragm | |||
| 192 | 193 | case domain.MergeRequestOpened: | |
| 193 | 194 | return l.mrNotice(ctx, ev.MergeRequest.Number, "a merge request was opened — reload") | |
| 194 | 195 | case domain.MergeRequestMerged: | |
| 196 | + | if l.view == "mr" && strconv.FormatInt(ev.MergeRequest.Number, 10) == l.n { | |
| 197 | + | r, err := http.NewRequestWithContext(ctx, http.MethodGet, mrHref(l.repo, ev.MergeRequest.Number), nil) | |
| 198 | + | if err != nil { | |
| 199 | + | return nil, err | |
| 200 | + | } | |
| 201 | + | r.SetPathValue("repo", l.repo) | |
| 202 | + | r.SetPathValue("n", l.n) | |
| 203 | + | c, _, err := l.s.mrPageView(ctx, r, false, true) | |
| 204 | + | if err != nil { | |
| 205 | + | return nil, err | |
| 206 | + | } | |
| 207 | + | f, err := renderFragment(ctx, "mr-mergeability", c) | |
| 208 | + | return []fragment{f}, err | |
| 209 | + | } | |
| 195 | 210 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was merged — reload") | |
| 196 | 211 | case domain.MergeRequestClosed: | |
| 197 | 212 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was closed — reload") | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "bytes" | 6 | "bytes" | ||
| 7 | "context" | 7 | "context" | ||
| 8 | "errors" | 8 | "errors" | ||
| 9 | + | "net/http" | |||
| 9 | "strconv" | 10 | "strconv" | ||
| 10 | "strings" | 11 | "strings" | ||
| 11 | "sync" | 12 | "sync" | ||
| @@ -192,6 +193,20 @@ func (l *liveStream) repoPage(ctx context.Context, we ports.WatchEvent) ([]fragm | |||||
| 192 | case domain.MergeRequestOpened: | 193 | case domain.MergeRequestOpened: | ||
| 193 | return l.mrNotice(ctx, ev.MergeRequest.Number, "a merge request was opened — reload") | 194 | return l.mrNotice(ctx, ev.MergeRequest.Number, "a merge request was opened — reload") | ||
| 194 | case domain.MergeRequestMerged: | 195 | case domain.MergeRequestMerged: | ||
| 196 | + | if l.view == "mr" && strconv.FormatInt(ev.MergeRequest.Number, 10) == l.n { | |||
| 197 | + | r, err := http.NewRequestWithContext(ctx, http.MethodGet, mrHref(l.repo, ev.MergeRequest.Number), nil) | |||
| 198 | + | if err != nil { | |||
| 199 | + | return nil, err | |||
| 200 | + | } | |||
| 201 | + | r.SetPathValue("repo", l.repo) | |||
| 202 | + | r.SetPathValue("n", l.n) | |||
| 203 | + | c, _, err := l.s.mrPageView(ctx, r, false, true) | |||
| 204 | + | if err != nil { | |||
| 205 | + | return nil, err | |||
| 206 | + | } | |||
| 207 | + | f, err := renderFragment(ctx, "mr-mergeability", c) | |||
| 208 | + | return []fragment{f}, err | |||
| 209 | + | } | |||
| 195 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was merged — reload") | 210 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was merged — reload") | ||
| 196 | case domain.MergeRequestClosed: | 211 | case domain.MergeRequestClosed: | ||
| 197 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was closed — reload") | 212 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was closed — reload") | ||
M internal/adapters/web/mr.go +7 -0
| @@ -558,6 +558,10 @@ func (s *Server) mrSince(ctx context.Context, r *http.Request) (templ.Component, | |||
| 558 | 558 | } | |
| 559 | 559 | ||
| 560 | 560 | func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ.Component, int, error) { | |
| 561 | + | return s.mrPageView(ctx, r, since, false) | |
| 562 | + | } | |
| 563 | + | ||
| 564 | + | func (s *Server) mrPageView(ctx context.Context, r *http.Request, since, fragment bool) (templ.Component, int, error) { | |
| 561 | 565 | if since && !SignedIn(ctx) { | |
| 562 | 566 | return nil, 0, errNotFound | |
| 563 | 567 | } | |
| @@ -649,6 +653,9 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||
| 649 | 653 | } | |
| 650 | 654 | } | |
| 651 | 655 | ||
| 656 | + | if fragment { | |
| 657 | + | return MRStateFragment(p), http.StatusOK, nil | |
| 658 | + | } | |
| 652 | 659 | l := s.repoLayout(ctx, d, "merge-requests") | |
| 653 | 660 | l.Title = p.Number + " " + p.Title + " · " + d.name() + " · " + s.instance | |
| 654 | 661 | if head != "" { | |
| @@ -558,6 +558,10 @@ func (s *Server) mrSince(ctx context.Context, r *http.Request) (templ.Component, | |||||
| 558 | } | 558 | } | ||
| 559 | 559 | ||||
| 560 | func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ.Component, int, error) { | 560 | func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ.Component, int, error) { | ||
| 561 | + | return s.mrPageView(ctx, r, since, false) | |||
| 562 | + | } | |||
| 563 | + | ||||
| 564 | + | func (s *Server) mrPageView(ctx context.Context, r *http.Request, since, fragment bool) (templ.Component, int, error) { | |||
| 561 | if since && !SignedIn(ctx) { | 565 | if since && !SignedIn(ctx) { | ||
| 562 | return nil, 0, errNotFound | 566 | return nil, 0, errNotFound | ||
| 563 | } | 567 | } | ||
| @@ -649,6 +653,9 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||||
| 649 | } | 653 | } | ||
| 650 | } | 654 | } | ||
| 651 | 655 | ||||
| 656 | + | if fragment { | |||
| 657 | + | return MRStateFragment(p), http.StatusOK, nil | |||
| 658 | + | } | |||
| 652 | l := s.repoLayout(ctx, d, "merge-requests") | 659 | l := s.repoLayout(ctx, d, "merge-requests") | ||
| 653 | l.Title = p.Number + " " + p.Title + " · " + d.name() + " · " + s.instance | 660 | l.Title = p.Number + " " + p.Title + " · " + d.name() + " · " + s.instance | ||
| 654 | if head != "" { | 661 | if head != "" { | ||
M internal/adapters/web/mr.templ +62 -29
| @@ -13,7 +13,7 @@ templ MRList(l Layout, p MRListPage) { | |||
| 13 | 13 | @Shell(l, mrListRail(p)) { | |
| 14 | 14 | if p.Live { | |
| 15 | 15 | @LiveSlot("mr-notice") | |
| 16 | - | <div id="mr-refresh-status" role="status" aria-live="polite"></div> | |
| 16 | + | <div id="mr-refresh-status" role="status" aria-live="polite"></div> | |
| 17 | 17 | } | |
| 18 | 18 | <div class="filter"> | |
| 19 | 19 | <label for="mr-filter" class="sr">filter</label> | |
| @@ -119,12 +119,7 @@ templ MergeRequest(l Layout, p MRPage) { | |||
| 119 | 119 | <h1><span class="mute">{ p.Number }</span> { p.Title }</h1> | |
| 120 | 120 | <dl class="kv show-kv"> | |
| 121 | 121 | <dt>state</dt> | |
| 122 | - | <dd class="mr-state" data-state={ p.State }> | |
| 123 | - | { p.State } | |
| 124 | - | if p.MergedShort != "" { | |
| 125 | - | { " · " }<a href={ templ.SafeURL(p.MergedHref) }>{ p.MergedShort }</a>{ " · " + p.MergeMode } | |
| 126 | - | } | |
| 127 | - | </dd> | |
| 122 | + | @mrState(p) | |
| 128 | 123 | <dt>branch</dt> | |
| 129 | 124 | <dd> | |
| 130 | 125 | @branchLink(p.Source, p.SourceHref) | |
| @@ -215,19 +210,19 @@ templ MergeRequest(l Layout, p MRPage) { | |||
| 215 | 210 | } | |
| 216 | 211 | </div> | |
| 217 | 212 | <div id="mr-comments"> | |
| 218 | - | <h2 class="rule-h" id="comments"><span>Comments</span><span class="count">{ itoa(p.NUnresolved) } open / { itoa(p.NComments) }</span></h2> | |
| 219 | - | for _, c := range p.Loose { | |
| 220 | - | @commentView(c, "") | |
| 221 | - | } | |
| 222 | - | for _, c := range p.General { | |
| 223 | - | @commentView(c, "") | |
| 224 | - | } | |
| 225 | - | if p.NComments == 0 { | |
| 226 | - | <p class="dim">no comments yet.</p> | |
| 227 | - | } | |
| 228 | - | if p.GeneralForm != nil { | |
| 229 | - | @commentForm(*p.GeneralForm, "") | |
| 230 | - | } | |
| 213 | + | <h2 class="rule-h" id="comments"><span>Comments</span><span class="count">{ itoa(p.NUnresolved) } open / { itoa(p.NComments) }</span></h2> | |
| 214 | + | for _, c := range p.Loose { | |
| 215 | + | @commentView(c, "") | |
| 216 | + | } | |
| 217 | + | for _, c := range p.General { | |
| 218 | + | @commentView(c, "") | |
| 219 | + | } | |
| 220 | + | if p.NComments == 0 { | |
| 221 | + | <p class="dim">no comments yet.</p> | |
| 222 | + | } | |
| 223 | + | if p.GeneralForm != nil { | |
| 224 | + | @commentForm(*p.GeneralForm, "") | |
| 225 | + | } | |
| 231 | 226 | </div> | |
| 232 | 227 | } | |
| 233 | 228 | } | |
| @@ -467,13 +462,7 @@ templ mrBar(p MRPage) { | |||
| 467 | 462 | <div class="diff-tools mr-bar" role="group" aria-label="Diff view and merge"> | |
| 468 | 463 | <span class="dim">{ plural(p.NFiles, "file", "files") } · <span class="plus">+{ itoa(p.Adds) }</span> <span class="minus">-{ itoa(p.Dels) }</span></span> | |
| 469 | 464 | @diffViewMenu(p.DiffView) | |
| 470 | - | <div class="mr-acts"> | |
| 471 | - | @MRChecksSum(p.ChecksSum) | |
| 472 | - | if p.Box != nil { | |
| 473 | - | @mergeMenu(p) | |
| 474 | - | } | |
| 475 | - | @moreMenu(p) | |
| 476 | - | </div> | |
| 465 | + | @mrActions(p) | |
| 477 | 466 | </div> | |
| 478 | 467 | } | |
| 479 | 468 | ||
| @@ -523,7 +512,15 @@ templ mergeMenu(p MRPage) { | |||
| 523 | 512 | </div> | |
| 524 | 513 | </form> | |
| 525 | 514 | } | |
| 526 | - | <form class="d-acts" method="post" action={ templ.SafeURL(p.Box.MergeAction) }> | |
| 515 | + | <form | |
| 516 | + | class="d-acts" | |
| 517 | + | method="post" | |
| 518 | + | action={ templ.SafeURL(p.Box.MergeAction) } | |
| 519 | + | hx-post={ p.Box.MergeAction } | |
| 520 | + | hx-swap="none" | |
| 521 | + | hx-sync="this:drop" | |
| 522 | + | hx-disable="find button[type=submit]" | |
| 523 | + | > | |
| 527 | 524 | <input type="hidden" name="head_id" value={ p.Box.Head }/> | |
| 528 | 525 | <fieldset class="d-modes"> | |
| 529 | 526 | <legend class="sr">merge mode</legend> | |
| @@ -557,9 +554,10 @@ templ mergeMenu(p MRPage) { | |||
| 557 | 554 | data-default | |
| 558 | 555 | disabled?={ !p.Box.CanMerge() } | |
| 559 | 556 | title={ p.Box.MergeTitle() } | |
| 560 | - | :disabled="!this.form.querySelector('input[name=mode]:checked:not(:disabled)')" | |
| 557 | + | :disabled="this.form.classList.contains('htmx-request') || !this.form.querySelector('input[name=mode]:checked:not(:disabled)')" | |
| 561 | 558 | :title="(this.form.querySelector('input[name=mode]:checked') || {}).title || ''" | |
| 562 | 559 | >Merge into { p.Target }</button> | |
| 560 | + | <span class="htmx-indicator" role="status">Merging…</span> | |
| 563 | 561 | </div> | |
| 564 | 562 | </form> | |
| 565 | 563 | if p.Box.CanAutoMerge { | |
| @@ -779,3 +777,38 @@ templ MRMergeChecks(c ChecksSummary) { | |||
| 779 | 777 | } | |
| 780 | 778 | </div> | |
| 781 | 779 | } | |
| 780 | + | ||
| 781 | + | // MRStateFragment updates only state and actions, leaving review drafts alone. | |
| 782 | + | templ MRStateFragment(p MRPage) { | |
| 783 | + | @mrState(p) | |
| 784 | + | @mrActions(p) | |
| 785 | + | } | |
| 786 | + | ||
| 787 | + | templ mrState(p MRPage) { | |
| 788 | + | <dd id="mr-state" hx-swap-oob="true" class="mr-state" data-state={ p.State }> | |
| 789 | + | { p.State } | |
| 790 | + | if p.MergedShort != "" { | |
| 791 | + | { " · " }<a href={ templ.SafeURL(p.MergedHref) }>{ p.MergedShort }</a>{ " · " + p.MergeMode } | |
| 792 | + | } | |
| 793 | + | </dd> | |
| 794 | + | } | |
| 795 | + | ||
| 796 | + | templ mrActions(p MRPage) { | |
| 797 | + | <div id="mr-actions" hx-swap-oob="true" class="mr-acts"> | |
| 798 | + | @MRChecksSum(p.ChecksSum) | |
| 799 | + | if p.Box != nil { | |
| 800 | + | @mergeMenu(p) | |
| 801 | + | } | |
| 802 | + | @moreMenu(p) | |
| 803 | + | if p.MergeError != "" { | |
| 804 | + | <p class="d-err" role="alert">{ p.MergeError }</p> | |
| 805 | + | } | |
| 806 | + | </div> | |
| 807 | + | } | |
| 808 | + | ||
| 809 | + | // MRActionStatus remains visible even when the request detail cannot be read. | |
| 810 | + | templ MRActionStatus(message, href string) { | |
| 811 | + | <div id="mr-actions" class="mr-acts" hx-swap-oob="true"> | |
| 812 | + | <p class="d-err" role="alert">{ message } <a href={ templ.SafeURL(href) }>reload</a></p> | |
| 813 | + | </div> | |
| 814 | + | } | |
| @@ -13,7 +13,7 @@ templ MRList(l Layout, p MRListPage) { | |||||
| 13 | @Shell(l, mrListRail(p)) { | 13 | @Shell(l, mrListRail(p)) { | ||
| 14 | if p.Live { | 14 | if p.Live { | ||
| 15 | @LiveSlot("mr-notice") | 15 | @LiveSlot("mr-notice") | ||
| 16 | - | <div id="mr-refresh-status" role="status" aria-live="polite"></div> | 16 | + | <div id="mr-refresh-status" role="status" aria-live="polite"></div> |
| 17 | } | 17 | } | ||
| 18 | <div class="filter"> | 18 | <div class="filter"> | ||
| 19 | <label for="mr-filter" class="sr">filter</label> | 19 | <label for="mr-filter" class="sr">filter</label> | ||
| @@ -119,12 +119,7 @@ templ MergeRequest(l Layout, p MRPage) { | |||||
| 119 | <h1><span class="mute">{ p.Number }</span> { p.Title }</h1> | 119 | <h1><span class="mute">{ p.Number }</span> { p.Title }</h1> | ||
| 120 | <dl class="kv show-kv"> | 120 | <dl class="kv show-kv"> | ||
| 121 | <dt>state</dt> | 121 | <dt>state</dt> | ||
| 122 | - | <dd class="mr-state" data-state={ p.State }> | 122 | + | @mrState(p) |
| 123 | - | { p.State } | |||
| 124 | - | if p.MergedShort != "" { | |||
| 125 | - | { " · " }<a href={ templ.SafeURL(p.MergedHref) }>{ p.MergedShort }</a>{ " · " + p.MergeMode } | |||
| 126 | - | } | |||
| 127 | - | </dd> | |||
| 128 | <dt>branch</dt> | 123 | <dt>branch</dt> | ||
| 129 | <dd> | 124 | <dd> | ||
| 130 | @branchLink(p.Source, p.SourceHref) | 125 | @branchLink(p.Source, p.SourceHref) | ||
| @@ -215,19 +210,19 @@ templ MergeRequest(l Layout, p MRPage) { | |||||
| 215 | } | 210 | } | ||
| 216 | </div> | 211 | </div> | ||
| 217 | <div id="mr-comments"> | 212 | <div id="mr-comments"> | ||
| 218 | - | <h2 class="rule-h" id="comments"><span>Comments</span><span class="count">{ itoa(p.NUnresolved) } open / { itoa(p.NComments) }</span></h2> | 213 | + | <h2 class="rule-h" id="comments"><span>Comments</span><span class="count">{ itoa(p.NUnresolved) } open / { itoa(p.NComments) }</span></h2> |
| 219 | - | for _, c := range p.Loose { | 214 | + | for _, c := range p.Loose { |
| 220 | - | @commentView(c, "") | 215 | + | @commentView(c, "") |
| 221 | - | } | 216 | + | } |
| 222 | - | for _, c := range p.General { | 217 | + | for _, c := range p.General { |
| 223 | - | @commentView(c, "") | 218 | + | @commentView(c, "") |
| 224 | - | } | 219 | + | } |
| 225 | - | if p.NComments == 0 { | 220 | + | if p.NComments == 0 { |
| 226 | - | <p class="dim">no comments yet.</p> | 221 | + | <p class="dim">no comments yet.</p> |
| 227 | - | } | 222 | + | } |
| 228 | - | if p.GeneralForm != nil { | 223 | + | if p.GeneralForm != nil { |
| 229 | - | @commentForm(*p.GeneralForm, "") | 224 | + | @commentForm(*p.GeneralForm, "") |
| 230 | - | } | 225 | + | } |
| 231 | </div> | 226 | </div> | ||
| 232 | } | 227 | } | ||
| 233 | } | 228 | } | ||
| @@ -467,13 +462,7 @@ templ mrBar(p MRPage) { | |||||
| 467 | <div class="diff-tools mr-bar" role="group" aria-label="Diff view and merge"> | 462 | <div class="diff-tools mr-bar" role="group" aria-label="Diff view and merge"> | ||
| 468 | <span class="dim">{ plural(p.NFiles, "file", "files") } · <span class="plus">+{ itoa(p.Adds) }</span> <span class="minus">-{ itoa(p.Dels) }</span></span> | 463 | <span class="dim">{ plural(p.NFiles, "file", "files") } · <span class="plus">+{ itoa(p.Adds) }</span> <span class="minus">-{ itoa(p.Dels) }</span></span> | ||
| 469 | @diffViewMenu(p.DiffView) | 464 | @diffViewMenu(p.DiffView) | ||
| 470 | - | <div class="mr-acts"> | 465 | + | @mrActions(p) |
| 471 | - | @MRChecksSum(p.ChecksSum) | |||
| 472 | - | if p.Box != nil { | |||
| 473 | - | @mergeMenu(p) | |||
| 474 | - | } | |||
| 475 | - | @moreMenu(p) | |||
| 476 | - | </div> | |||
| 477 | </div> | 466 | </div> | ||
| 478 | } | 467 | } | ||
| 479 | 468 | ||||
| @@ -523,7 +512,15 @@ templ mergeMenu(p MRPage) { | |||||
| 523 | </div> | 512 | </div> | ||
| 524 | </form> | 513 | </form> | ||
| 525 | } | 514 | } | ||
| 526 | - | <form class="d-acts" method="post" action={ templ.SafeURL(p.Box.MergeAction) }> | 515 | + | <form |
| 516 | + | class="d-acts" | |||
| 517 | + | method="post" | |||
| 518 | + | action={ templ.SafeURL(p.Box.MergeAction) } | |||
| 519 | + | hx-post={ p.Box.MergeAction } | |||
| 520 | + | hx-swap="none" | |||
| 521 | + | hx-sync="this:drop" | |||
| 522 | + | hx-disable="find button[type=submit]" | |||
| 523 | + | > | |||
| 527 | <input type="hidden" name="head_id" value={ p.Box.Head }/> | 524 | <input type="hidden" name="head_id" value={ p.Box.Head }/> | ||
| 528 | <fieldset class="d-modes"> | 525 | <fieldset class="d-modes"> | ||
| 529 | <legend class="sr">merge mode</legend> | 526 | <legend class="sr">merge mode</legend> | ||
| @@ -557,9 +554,10 @@ templ mergeMenu(p MRPage) { | |||||
| 557 | data-default | 554 | data-default | ||
| 558 | disabled?={ !p.Box.CanMerge() } | 555 | disabled?={ !p.Box.CanMerge() } | ||
| 559 | title={ p.Box.MergeTitle() } | 556 | title={ p.Box.MergeTitle() } | ||
| 560 | - | :disabled="!this.form.querySelector('input[name=mode]:checked:not(:disabled)')" | 557 | + | :disabled="this.form.classList.contains('htmx-request') || !this.form.querySelector('input[name=mode]:checked:not(:disabled)')" |
| 561 | :title="(this.form.querySelector('input[name=mode]:checked') || {}).title || ''" | 558 | :title="(this.form.querySelector('input[name=mode]:checked') || {}).title || ''" | ||
| 562 | >Merge into { p.Target }</button> | 559 | >Merge into { p.Target }</button> | ||
| 560 | + | <span class="htmx-indicator" role="status">Merging…</span> | |||
| 563 | </div> | 561 | </div> | ||
| 564 | </form> | 562 | </form> | ||
| 565 | if p.Box.CanAutoMerge { | 563 | if p.Box.CanAutoMerge { | ||
| @@ -779,3 +777,38 @@ templ MRMergeChecks(c ChecksSummary) { | |||||
| 779 | } | 777 | } | ||
| 780 | </div> | 778 | </div> | ||
| 781 | } | 779 | } | ||
| 780 | + | ||||
| 781 | + | // MRStateFragment updates only state and actions, leaving review drafts alone. | |||
| 782 | + | templ MRStateFragment(p MRPage) { | |||
| 783 | + | @mrState(p) | |||
| 784 | + | @mrActions(p) | |||
| 785 | + | } | |||
| 786 | + | ||||
| 787 | + | templ mrState(p MRPage) { | |||
| 788 | + | <dd id="mr-state" hx-swap-oob="true" class="mr-state" data-state={ p.State }> | |||
| 789 | + | { p.State } | |||
| 790 | + | if p.MergedShort != "" { | |||
| 791 | + | { " · " }<a href={ templ.SafeURL(p.MergedHref) }>{ p.MergedShort }</a>{ " · " + p.MergeMode } | |||
| 792 | + | } | |||
| 793 | + | </dd> | |||
| 794 | + | } | |||
| 795 | + | ||||
| 796 | + | templ mrActions(p MRPage) { | |||
| 797 | + | <div id="mr-actions" hx-swap-oob="true" class="mr-acts"> | |||
| 798 | + | @MRChecksSum(p.ChecksSum) | |||
| 799 | + | if p.Box != nil { | |||
| 800 | + | @mergeMenu(p) | |||
| 801 | + | } | |||
| 802 | + | @moreMenu(p) | |||
| 803 | + | if p.MergeError != "" { | |||
| 804 | + | <p class="d-err" role="alert">{ p.MergeError }</p> | |||
| 805 | + | } | |||
| 806 | + | </div> | |||
| 807 | + | } | |||
| 808 | + | ||||
| 809 | + | // MRActionStatus remains visible even when the request detail cannot be read. | |||
| 810 | + | templ MRActionStatus(message, href string) { | |||
| 811 | + | <div id="mr-actions" class="mr-acts" hx-swap-oob="true"> | |||
| 812 | + | <p class="d-err" role="alert">{ message } <a href={ templ.SafeURL(href) }>reload</a></p> | |||
| 813 | + | </div> | |||
| 814 | + | } | |||
A internal/adapters/web/mr_async_test.go +127 -0
| @@ -0,0 +1,127 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package web | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "errors" | |
| 8 | + | "net/http" | |
| 9 | + | "net/http/httptest" | |
| 10 | + | "strings" | |
| 11 | + | "testing" | |
| 12 | + | "time" | |
| 13 | + | ||
| 14 | + | "github.com/stretchr/testify/assert" | |
| 15 | + | "github.com/stretchr/testify/mock" | |
| 16 | + | "github.com/stretchr/testify/require" | |
| 17 | + | ||
| 18 | + | "bnema.dev/ermite/internal/domain" | |
| 19 | + | "bnema.dev/ermite/internal/ports" | |
| 20 | + | "bnema.dev/ermite/internal/ports/mocks" | |
| 21 | + | ) | |
| 22 | + | ||
| 23 | + | func asyncMergeServer(t *testing.T, state domain.MRState) (*Server, *mocks.MergeRequestService) { | |
| 24 | + | t.Helper() | |
| 25 | + | svc := mocks.NewMergeRequestService(t) | |
| 26 | + | browse := mocks.NewBrowseService(t) | |
| 27 | + | browse.EXPECT().ListRefs(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(ports.RefPage{Refs: []domain.Ref{{Kind: domain.RefKindBranch, ShortName: "feature"}}}, nil).Maybe() | |
| 28 | + | svc.EXPECT().Get(mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), mock.Anything).Return(ports.MergeRequestDetail{ | |
| 29 | + | MergeRequest: domain.MergeRequest{Repo: repoName(t, "demo"), Number: 3, State: state, HeadID: objectID(t, strings.Repeat("a", domain.ObjectIDLen)), Source: "feature", Target: "main"}, | |
| 30 | + | Mergeability: domain.Mergeability{State: domain.MergeabilityUpToDate}, | |
| 31 | + | }, nil).Maybe() | |
| 32 | + | return newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "demo")), Browse: browse, MergeRequests: svc, Now: time.Now}), svc | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | func TestAsyncMergePost(t *testing.T) { | |
| 36 | + | for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash, domain.MergeModeMerge} { | |
| 37 | + | for _, hx := range []bool{false, true} { | |
| 38 | + | t.Run(mode.String()+"/"+boolText(hx), func(t *testing.T) { | |
| 39 | + | s, svc := asyncMergeServer(t, domain.MRStateMerged) | |
| 40 | + | svc.EXPECT().Merge(mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), ports.MergeParams{Mode: mode}).Return(ports.MergeResult{}, nil).Once() | |
| 41 | + | r := autoMergeRequest("/demo/merge-requests/3/merge", "mode="+mode.String()) | |
| 42 | + | if hx { | |
| 43 | + | r.Header.Set("HX-Request", "true") | |
| 44 | + | } | |
| 45 | + | w := httptest.NewRecorder() | |
| 46 | + | s.mrAction(s.mrMerge)(w, r) | |
| 47 | + | if !hx { | |
| 48 | + | assert.Equal(t, http.StatusSeeOther, w.Code) | |
| 49 | + | return | |
| 50 | + | } | |
| 51 | + | assert.Equal(t, http.StatusOK, w.Code) | |
| 52 | + | assert.Empty(t, w.Header().Get("Location")) | |
| 53 | + | assert.Contains(t, w.Body.String(), `id="mr-state"`) | |
| 54 | + | assert.Contains(t, w.Body.String(), `data-state="merged"`) | |
| 55 | + | assert.Contains(t, w.Body.String(), `id="mr-actions"`) | |
| 56 | + | assert.NotContains(t, w.Body.String(), "<html") | |
| 57 | + | assert.NotContains(t, w.Body.String(), `id="merge-menu"`) | |
| 58 | + | }) | |
| 59 | + | } | |
| 60 | + | } | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | func TestAsyncMergeRefusal(t *testing.T) { | |
| 64 | + | s, svc := asyncMergeServer(t, domain.MRStateOpen) | |
| 65 | + | svc.EXPECT().Merge(mock.Anything, mock.Anything, mock.Anything, int64(3), mock.Anything).Return(ports.MergeResult{}, domain.ErrConflict) | |
| 66 | + | r := autoMergeRequest("/demo/merge-requests/3/merge", "mode=squash") | |
| 67 | + | r = r.WithContext(context.WithValue(r.Context(), signedInKey{}, struct{}{})) | |
| 68 | + | r.Header.Set("HX-Request", "true") | |
| 69 | + | w := httptest.NewRecorder() | |
| 70 | + | s.mrAction(s.mrMerge)(w, r) | |
| 71 | + | assert.Equal(t, http.StatusConflict, w.Code) | |
| 72 | + | assert.Contains(t, w.Body.String(), `role="alert"`) | |
| 73 | + | assert.Contains(t, w.Body.String(), domain.ErrConflict.Error()) | |
| 74 | + | assert.Contains(t, w.Body.String(), `hx-post="/demo/merge-requests/3/merge"`) | |
| 75 | + | assert.Contains(t, w.Body.String(), `hx-sync="this:drop"`) | |
| 76 | + | assert.Contains(t, w.Body.String(), "Merging…") | |
| 77 | + | assert.NotContains(t, w.Body.String(), "<html") | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func TestAsyncMergeLive(t *testing.T) { | |
| 81 | + | s, _ := asyncMergeServer(t, domain.MRStateMerged) | |
| 82 | + | l := liveStream{s: s, repo: "demo", view: "mr", n: "3"} | |
| 83 | + | fs, err := l.repoPage(t.Context(), watchOf(domain.MergeRequestMerged{MergeRequest: domain.MergeRequest{Number: 3}})) | |
| 84 | + | require.NoError(t, err) | |
| 85 | + | require.Len(t, fs, 1) | |
| 86 | + | assert.Equal(t, "mr-mergeability", fs[0].event) | |
| 87 | + | assert.Contains(t, fs[0].html, `data-state="merged"`) | |
| 88 | + | assert.Contains(t, fs[0].html, `id="mr-actions"`) | |
| 89 | + | assert.False(t, strings.Contains(fs[0].html, "reload")) | |
| 90 | + | fs, err = l.repoPage(t.Context(), watchOf(domain.MergeRequestMerged{MergeRequest: domain.MergeRequest{Number: 4}})) | |
| 91 | + | require.NoError(t, err) | |
| 92 | + | assert.Empty(t, fs) | |
| 93 | + | } | |
| 94 | + | ||
| 95 | + | func TestAsyncMergeOtherFailures(t *testing.T) { | |
| 96 | + | for _, tt := range []struct { | |
| 97 | + | name string | |
| 98 | + | mergeErr error | |
| 99 | + | readFails bool | |
| 100 | + | status int | |
| 101 | + | message string | |
| 102 | + | }{ | |
| 103 | + | {"forbidden", domain.ErrForbidden, false, http.StatusForbidden, "the session is not allowed"}, | |
| 104 | + | {"internal", errors.New("private backend detail"), false, http.StatusInternalServerError, "reload to check its state"}, | |
| 105 | + | {"post-merge read", nil, true, http.StatusOK, "merged, but the updated page could not be loaded"}, | |
| 106 | + | } { | |
| 107 | + | t.Run(tt.name, func(t *testing.T) { | |
| 108 | + | s, svc := asyncMergeServer(t, domain.MRStateMerged) | |
| 109 | + | if tt.readFails { | |
| 110 | + | svc.On("Get", mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), mock.Anything).Unset() | |
| 111 | + | svc.EXPECT().Get(mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), mock.Anything).Return(ports.MergeRequestDetail{}, errors.New("private read detail")).Once() | |
| 112 | + | } | |
| 113 | + | svc.EXPECT().Merge(mock.Anything, mock.Anything, mock.Anything, int64(3), mock.Anything).Return(ports.MergeResult{}, tt.mergeErr).Once() | |
| 114 | + | r := autoMergeRequest("/demo/merge-requests/3/merge", "mode=squash") | |
| 115 | + | r.Header.Set("HX-Request", "true") | |
| 116 | + | w := httptest.NewRecorder() | |
| 117 | + | s.mrAction(s.mrMerge)(w, r) | |
| 118 | + | assert.Equal(t, tt.status, w.Code) | |
| 119 | + | assert.Contains(t, w.Body.String(), `id="mr-actions"`) | |
| 120 | + | assert.Contains(t, w.Body.String(), `hx-swap-oob="true"`) | |
| 121 | + | assert.Contains(t, w.Body.String(), `role="alert"`) | |
| 122 | + | assert.Contains(t, w.Body.String(), tt.message) | |
| 123 | + | assert.NotContains(t, w.Body.String(), "private") | |
| 124 | + | assert.NotContains(t, w.Body.String(), "<html") | |
| 125 | + | }) | |
| 126 | + | } | |
| 127 | + | } | |
| @@ -0,0 +1,127 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package web | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "errors" | |||
| 8 | + | "net/http" | |||
| 9 | + | "net/http/httptest" | |||
| 10 | + | "strings" | |||
| 11 | + | "testing" | |||
| 12 | + | "time" | |||
| 13 | + | ||||
| 14 | + | "github.com/stretchr/testify/assert" | |||
| 15 | + | "github.com/stretchr/testify/mock" | |||
| 16 | + | "github.com/stretchr/testify/require" | |||
| 17 | + | ||||
| 18 | + | "bnema.dev/ermite/internal/domain" | |||
| 19 | + | "bnema.dev/ermite/internal/ports" | |||
| 20 | + | "bnema.dev/ermite/internal/ports/mocks" | |||
| 21 | + | ) | |||
| 22 | + | ||||
| 23 | + | func asyncMergeServer(t *testing.T, state domain.MRState) (*Server, *mocks.MergeRequestService) { | |||
| 24 | + | t.Helper() | |||
| 25 | + | svc := mocks.NewMergeRequestService(t) | |||
| 26 | + | browse := mocks.NewBrowseService(t) | |||
| 27 | + | browse.EXPECT().ListRefs(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(ports.RefPage{Refs: []domain.Ref{{Kind: domain.RefKindBranch, ShortName: "feature"}}}, nil).Maybe() | |||
| 28 | + | svc.EXPECT().Get(mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), mock.Anything).Return(ports.MergeRequestDetail{ | |||
| 29 | + | MergeRequest: domain.MergeRequest{Repo: repoName(t, "demo"), Number: 3, State: state, HeadID: objectID(t, strings.Repeat("a", domain.ObjectIDLen)), Source: "feature", Target: "main"}, | |||
| 30 | + | Mergeability: domain.Mergeability{State: domain.MergeabilityUpToDate}, | |||
| 31 | + | }, nil).Maybe() | |||
| 32 | + | return newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "demo")), Browse: browse, MergeRequests: svc, Now: time.Now}), svc | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | func TestAsyncMergePost(t *testing.T) { | |||
| 36 | + | for _, mode := range []domain.MergeMode{domain.MergeModeFastForward, domain.MergeModeSquash, domain.MergeModeMerge} { | |||
| 37 | + | for _, hx := range []bool{false, true} { | |||
| 38 | + | t.Run(mode.String()+"/"+boolText(hx), func(t *testing.T) { | |||
| 39 | + | s, svc := asyncMergeServer(t, domain.MRStateMerged) | |||
| 40 | + | svc.EXPECT().Merge(mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), ports.MergeParams{Mode: mode}).Return(ports.MergeResult{}, nil).Once() | |||
| 41 | + | r := autoMergeRequest("/demo/merge-requests/3/merge", "mode="+mode.String()) | |||
| 42 | + | if hx { | |||
| 43 | + | r.Header.Set("HX-Request", "true") | |||
| 44 | + | } | |||
| 45 | + | w := httptest.NewRecorder() | |||
| 46 | + | s.mrAction(s.mrMerge)(w, r) | |||
| 47 | + | if !hx { | |||
| 48 | + | assert.Equal(t, http.StatusSeeOther, w.Code) | |||
| 49 | + | return | |||
| 50 | + | } | |||
| 51 | + | assert.Equal(t, http.StatusOK, w.Code) | |||
| 52 | + | assert.Empty(t, w.Header().Get("Location")) | |||
| 53 | + | assert.Contains(t, w.Body.String(), `id="mr-state"`) | |||
| 54 | + | assert.Contains(t, w.Body.String(), `data-state="merged"`) | |||
| 55 | + | assert.Contains(t, w.Body.String(), `id="mr-actions"`) | |||
| 56 | + | assert.NotContains(t, w.Body.String(), "<html") | |||
| 57 | + | assert.NotContains(t, w.Body.String(), `id="merge-menu"`) | |||
| 58 | + | }) | |||
| 59 | + | } | |||
| 60 | + | } | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | func TestAsyncMergeRefusal(t *testing.T) { | |||
| 64 | + | s, svc := asyncMergeServer(t, domain.MRStateOpen) | |||
| 65 | + | svc.EXPECT().Merge(mock.Anything, mock.Anything, mock.Anything, int64(3), mock.Anything).Return(ports.MergeResult{}, domain.ErrConflict) | |||
| 66 | + | r := autoMergeRequest("/demo/merge-requests/3/merge", "mode=squash") | |||
| 67 | + | r = r.WithContext(context.WithValue(r.Context(), signedInKey{}, struct{}{})) | |||
| 68 | + | r.Header.Set("HX-Request", "true") | |||
| 69 | + | w := httptest.NewRecorder() | |||
| 70 | + | s.mrAction(s.mrMerge)(w, r) | |||
| 71 | + | assert.Equal(t, http.StatusConflict, w.Code) | |||
| 72 | + | assert.Contains(t, w.Body.String(), `role="alert"`) | |||
| 73 | + | assert.Contains(t, w.Body.String(), domain.ErrConflict.Error()) | |||
| 74 | + | assert.Contains(t, w.Body.String(), `hx-post="/demo/merge-requests/3/merge"`) | |||
| 75 | + | assert.Contains(t, w.Body.String(), `hx-sync="this:drop"`) | |||
| 76 | + | assert.Contains(t, w.Body.String(), "Merging…") | |||
| 77 | + | assert.NotContains(t, w.Body.String(), "<html") | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | func TestAsyncMergeLive(t *testing.T) { | |||
| 81 | + | s, _ := asyncMergeServer(t, domain.MRStateMerged) | |||
| 82 | + | l := liveStream{s: s, repo: "demo", view: "mr", n: "3"} | |||
| 83 | + | fs, err := l.repoPage(t.Context(), watchOf(domain.MergeRequestMerged{MergeRequest: domain.MergeRequest{Number: 3}})) | |||
| 84 | + | require.NoError(t, err) | |||
| 85 | + | require.Len(t, fs, 1) | |||
| 86 | + | assert.Equal(t, "mr-mergeability", fs[0].event) | |||
| 87 | + | assert.Contains(t, fs[0].html, `data-state="merged"`) | |||
| 88 | + | assert.Contains(t, fs[0].html, `id="mr-actions"`) | |||
| 89 | + | assert.False(t, strings.Contains(fs[0].html, "reload")) | |||
| 90 | + | fs, err = l.repoPage(t.Context(), watchOf(domain.MergeRequestMerged{MergeRequest: domain.MergeRequest{Number: 4}})) | |||
| 91 | + | require.NoError(t, err) | |||
| 92 | + | assert.Empty(t, fs) | |||
| 93 | + | } | |||
| 94 | + | ||||
| 95 | + | func TestAsyncMergeOtherFailures(t *testing.T) { | |||
| 96 | + | for _, tt := range []struct { | |||
| 97 | + | name string | |||
| 98 | + | mergeErr error | |||
| 99 | + | readFails bool | |||
| 100 | + | status int | |||
| 101 | + | message string | |||
| 102 | + | }{ | |||
| 103 | + | {"forbidden", domain.ErrForbidden, false, http.StatusForbidden, "the session is not allowed"}, | |||
| 104 | + | {"internal", errors.New("private backend detail"), false, http.StatusInternalServerError, "reload to check its state"}, | |||
| 105 | + | {"post-merge read", nil, true, http.StatusOK, "merged, but the updated page could not be loaded"}, | |||
| 106 | + | } { | |||
| 107 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 108 | + | s, svc := asyncMergeServer(t, domain.MRStateMerged) | |||
| 109 | + | if tt.readFails { | |||
| 110 | + | svc.On("Get", mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), mock.Anything).Unset() | |||
| 111 | + | svc.EXPECT().Get(mock.Anything, mock.Anything, repoName(t, "demo"), int64(3), mock.Anything).Return(ports.MergeRequestDetail{}, errors.New("private read detail")).Once() | |||
| 112 | + | } | |||
| 113 | + | svc.EXPECT().Merge(mock.Anything, mock.Anything, mock.Anything, int64(3), mock.Anything).Return(ports.MergeResult{}, tt.mergeErr).Once() | |||
| 114 | + | r := autoMergeRequest("/demo/merge-requests/3/merge", "mode=squash") | |||
| 115 | + | r.Header.Set("HX-Request", "true") | |||
| 116 | + | w := httptest.NewRecorder() | |||
| 117 | + | s.mrAction(s.mrMerge)(w, r) | |||
| 118 | + | assert.Equal(t, tt.status, w.Code) | |||
| 119 | + | assert.Contains(t, w.Body.String(), `id="mr-actions"`) | |||
| 120 | + | assert.Contains(t, w.Body.String(), `hx-swap-oob="true"`) | |||
| 121 | + | assert.Contains(t, w.Body.String(), `role="alert"`) | |||
| 122 | + | assert.Contains(t, w.Body.String(), tt.message) | |||
| 123 | + | assert.NotContains(t, w.Body.String(), "private") | |||
| 124 | + | assert.NotContains(t, w.Body.String(), "<html") | |||
| 125 | + | }) | |||
| 126 | + | } | |||
| 127 | + | } | |||
M internal/adapters/web/mr_post.go +33 -1
| @@ -68,6 +68,20 @@ func (s *Server) mrAction(h func(ctx context.Context, r *http.Request) (to strin | |||
| 68 | 68 | s.actionFailed(ctx, w, r, err, back) | |
| 69 | 69 | return | |
| 70 | 70 | } | |
| 71 | + | if isHXMerge(r) { | |
| 72 | + | c, _, err := s.mrPageView(ctx, r, false, true) | |
| 73 | + | if err != nil { | |
| 74 | + | id := requestID() | |
| 75 | + | log := zerowrap.FromCtx(ctx) | |
| 76 | + | log.Error().Err(err).Str("request_id", id).Msg("web merged state failed") | |
| 77 | + | w.Header().Set("Cache-Control", "no-store") | |
| 78 | + | s.write(ctx, w, r, http.StatusOK, MRActionStatus("merged, but the updated page could not be loaded. reload to see the result. reference: "+id, back)) | |
| 79 | + | return | |
| 80 | + | } | |
| 81 | + | w.Header().Set("Cache-Control", "no-store") | |
| 82 | + | s.write(ctx, w, r, http.StatusOK, c) | |
| 83 | + | return | |
| 84 | + | } | |
| 71 | 85 | seeOther(w, r, to) | |
| 72 | 86 | } | |
| 73 | 87 | } | |
| @@ -100,7 +114,7 @@ func mergeErrorOf(ctx context.Context) string { | |||
| 100 | 114 | // It reports false when the page cannot be built; the caller then answers as | |
| 101 | 115 | // for any refused action. | |
| 102 | 116 | func (s *Server) rerenderMR(ctx context.Context, w http.ResponseWriter, r *http.Request, re *rerender) bool { | |
| 103 | - | c, _, err := s.mrPage(withMergeError(ctx, re.msg), r, false) | |
| 117 | + | c, _, err := s.mrPageView(withMergeError(ctx, re.msg), r, false, isHXMerge(r)) | |
| 104 | 118 | if err != nil { | |
| 105 | 119 | return false | |
| 106 | 120 | } | |
| @@ -120,6 +134,10 @@ func (s *Server) actionFailed(ctx context.Context, w http.ResponseWriter, r *htt | |||
| 120 | 134 | // The browser is gone. | |
| 121 | 135 | case errors.Is(err, domain.ErrNotFound): | |
| 122 | 136 | w.Header().Set("Cache-Control", "no-store") | |
| 137 | + | if isHXMerge(r) { | |
| 138 | + | s.write(ctx, w, r, http.StatusNotFound, MRActionStatus("the merge request was not found. reload to check its state.", back)) | |
| 139 | + | return | |
| 140 | + | } | |
| 123 | 141 | s.write(ctx, w, r, http.StatusNotFound, s.notFoundPage()) | |
| 124 | 142 | case errors.Is(err, domain.ErrAlreadyExists): | |
| 125 | 143 | s.reject(ctx, w, r, http.StatusConflict, "already there", rule(msg, "this already exists."), back) | |
| @@ -143,6 +161,10 @@ func (s *Server) internalFailure(ctx context.Context, w http.ResponseWriter, r * | |||
| 143 | 161 | log := zerowrap.FromCtx(ctx) | |
| 144 | 162 | log.Error().Err(err).Str("request_id", id).Str("path", logPath(r.URL.Path)).Msg("web action failed") | |
| 145 | 163 | w.Header().Set("Cache-Control", "no-store") | |
| 164 | + | if isHXMerge(r) { | |
| 165 | + | s.write(ctx, w, r, http.StatusInternalServerError, MRActionStatus("the merge could not be completed. reload to check its state. reference: "+id, strings.TrimSuffix(r.URL.Path, "/merge"))) | |
| 166 | + | return | |
| 167 | + | } | |
| 146 | 168 | s.write(ctx, w, r, http.StatusInternalServerError, s.errorPage(id)) | |
| 147 | 169 | } | |
| 148 | 170 | ||
| @@ -161,6 +183,11 @@ func (s *Server) reject(ctx context.Context, w http.ResponseWriter, r *http.Requ | |||
| 161 | 183 | if back == "" { | |
| 162 | 184 | back = "/" | |
| 163 | 185 | } | |
| 186 | + | if isHXMerge(r) { | |
| 187 | + | w.Header().Set("Cache-Control", "no-store") | |
| 188 | + | s.write(ctx, w, r, status, MRActionStatus(line, back)) | |
| 189 | + | return | |
| 190 | + | } | |
| 164 | 191 | l := s.stateLayout(title, []Crumb{{Label: title, Current: true}}) | |
| 165 | 192 | w.Header().Set("Cache-Control", "no-store") | |
| 166 | 193 | s.write(ctx, w, r, status, State(l, StatePage{ | |
| @@ -523,3 +550,8 @@ func (s *Server) mrDeleteComment(ctx context.Context, r *http.Request) (string, | |||
| 523 | 550 | c, err := s.cfg.MergeRequests.DeleteComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id"))) | |
| 524 | 551 | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | |
| 525 | 552 | } | |
| 553 | + | ||
| 554 | + | // isHXMerge limits fragment responses to the progressively enhanced merge form. | |
| 555 | + | func isHXMerge(r *http.Request) bool { | |
| 556 | + | return r.Header.Get("HX-Request") == "true" && strings.HasSuffix(r.URL.Path, "/merge") | |
| 557 | + | } | |
| @@ -68,6 +68,20 @@ func (s *Server) mrAction(h func(ctx context.Context, r *http.Request) (to strin | |||||
| 68 | s.actionFailed(ctx, w, r, err, back) | 68 | s.actionFailed(ctx, w, r, err, back) | ||
| 69 | return | 69 | return | ||
| 70 | } | 70 | } | ||
| 71 | + | if isHXMerge(r) { | |||
| 72 | + | c, _, err := s.mrPageView(ctx, r, false, true) | |||
| 73 | + | if err != nil { | |||
| 74 | + | id := requestID() | |||
| 75 | + | log := zerowrap.FromCtx(ctx) | |||
| 76 | + | log.Error().Err(err).Str("request_id", id).Msg("web merged state failed") | |||
| 77 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 78 | + | s.write(ctx, w, r, http.StatusOK, MRActionStatus("merged, but the updated page could not be loaded. reload to see the result. reference: "+id, back)) | |||
| 79 | + | return | |||
| 80 | + | } | |||
| 81 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 82 | + | s.write(ctx, w, r, http.StatusOK, c) | |||
| 83 | + | return | |||
| 84 | + | } | |||
| 71 | seeOther(w, r, to) | 85 | seeOther(w, r, to) | ||
| 72 | } | 86 | } | ||
| 73 | } | 87 | } | ||
| @@ -100,7 +114,7 @@ func mergeErrorOf(ctx context.Context) string { | |||||
| 100 | // It reports false when the page cannot be built; the caller then answers as | 114 | // It reports false when the page cannot be built; the caller then answers as | ||
| 101 | // for any refused action. | 115 | // for any refused action. | ||
| 102 | func (s *Server) rerenderMR(ctx context.Context, w http.ResponseWriter, r *http.Request, re *rerender) bool { | 116 | func (s *Server) rerenderMR(ctx context.Context, w http.ResponseWriter, r *http.Request, re *rerender) bool { | ||
| 103 | - | c, _, err := s.mrPage(withMergeError(ctx, re.msg), r, false) | 117 | + | c, _, err := s.mrPageView(withMergeError(ctx, re.msg), r, false, isHXMerge(r)) |
| 104 | if err != nil { | 118 | if err != nil { | ||
| 105 | return false | 119 | return false | ||
| 106 | } | 120 | } | ||
| @@ -120,6 +134,10 @@ func (s *Server) actionFailed(ctx context.Context, w http.ResponseWriter, r *htt | |||||
| 120 | // The browser is gone. | 134 | // The browser is gone. | ||
| 121 | case errors.Is(err, domain.ErrNotFound): | 135 | case errors.Is(err, domain.ErrNotFound): | ||
| 122 | w.Header().Set("Cache-Control", "no-store") | 136 | w.Header().Set("Cache-Control", "no-store") | ||
| 137 | + | if isHXMerge(r) { | |||
| 138 | + | s.write(ctx, w, r, http.StatusNotFound, MRActionStatus("the merge request was not found. reload to check its state.", back)) | |||
| 139 | + | return | |||
| 140 | + | } | |||
| 123 | s.write(ctx, w, r, http.StatusNotFound, s.notFoundPage()) | 141 | s.write(ctx, w, r, http.StatusNotFound, s.notFoundPage()) | ||
| 124 | case errors.Is(err, domain.ErrAlreadyExists): | 142 | case errors.Is(err, domain.ErrAlreadyExists): | ||
| 125 | s.reject(ctx, w, r, http.StatusConflict, "already there", rule(msg, "this already exists."), back) | 143 | s.reject(ctx, w, r, http.StatusConflict, "already there", rule(msg, "this already exists."), back) | ||
| @@ -143,6 +161,10 @@ func (s *Server) internalFailure(ctx context.Context, w http.ResponseWriter, r * | |||||
| 143 | log := zerowrap.FromCtx(ctx) | 161 | log := zerowrap.FromCtx(ctx) | ||
| 144 | log.Error().Err(err).Str("request_id", id).Str("path", logPath(r.URL.Path)).Msg("web action failed") | 162 | log.Error().Err(err).Str("request_id", id).Str("path", logPath(r.URL.Path)).Msg("web action failed") | ||
| 145 | w.Header().Set("Cache-Control", "no-store") | 163 | w.Header().Set("Cache-Control", "no-store") | ||
| 164 | + | if isHXMerge(r) { | |||
| 165 | + | s.write(ctx, w, r, http.StatusInternalServerError, MRActionStatus("the merge could not be completed. reload to check its state. reference: "+id, strings.TrimSuffix(r.URL.Path, "/merge"))) | |||
| 166 | + | return | |||
| 167 | + | } | |||
| 146 | s.write(ctx, w, r, http.StatusInternalServerError, s.errorPage(id)) | 168 | s.write(ctx, w, r, http.StatusInternalServerError, s.errorPage(id)) | ||
| 147 | } | 169 | } | ||
| 148 | 170 | ||||
| @@ -161,6 +183,11 @@ func (s *Server) reject(ctx context.Context, w http.ResponseWriter, r *http.Requ | |||||
| 161 | if back == "" { | 183 | if back == "" { | ||
| 162 | back = "/" | 184 | back = "/" | ||
| 163 | } | 185 | } | ||
| 186 | + | if isHXMerge(r) { | |||
| 187 | + | w.Header().Set("Cache-Control", "no-store") | |||
| 188 | + | s.write(ctx, w, r, status, MRActionStatus(line, back)) | |||
| 189 | + | return | |||
| 190 | + | } | |||
| 164 | l := s.stateLayout(title, []Crumb{{Label: title, Current: true}}) | 191 | l := s.stateLayout(title, []Crumb{{Label: title, Current: true}}) | ||
| 165 | w.Header().Set("Cache-Control", "no-store") | 192 | w.Header().Set("Cache-Control", "no-store") | ||
| 166 | s.write(ctx, w, r, status, State(l, StatePage{ | 193 | s.write(ctx, w, r, status, State(l, StatePage{ | ||
| @@ -523,3 +550,8 @@ func (s *Server) mrDeleteComment(ctx context.Context, r *http.Request) (string, | |||||
| 523 | c, err := s.cfg.MergeRequests.DeleteComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id"))) | 550 | c, err := s.cfg.MergeRequests.DeleteComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id"))) | ||
| 524 | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | 551 | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | ||
| 525 | } | 552 | } | ||
| 553 | + | ||||
| 554 | + | // isHXMerge limits fragment responses to the progressively enhanced merge form. | |||
| 555 | + | func isHXMerge(r *http.Request) bool { | |||
| 556 | + | return r.Header.Get("HX-Request") == "true" && strings.HasSuffix(r.URL.Path, "/merge") | |||
| 557 | + | } | |||
M internal/adapters/web/mr_templ.go +1226 -1034 generated file: collapsed
M internal/adapters/web/static/css/60-mr.css +2 -0
| @@ -1,6 +1,8 @@ | |||
| 1 | 1 | @layer components { | |
| 2 | 2 | /* merge requests: the review page. Hue only where it carries a verdict. */ | |
| 3 | 3 | .mr-h .show-kv { margin-top: 0; } | |
| 4 | + | #merge-menu .htmx-indicator { display: none; } | |
| 5 | + | #merge-menu .htmx-request .htmx-indicator { display: inline; } | |
| 4 | 6 | /* one card: the header (title, facts) a shade lighter, the description below */ | |
| 5 | 7 | .mr-card { | |
| 6 | 8 | background: var(--bg-surface); border: 1px solid var(--border); border-radius: var(--r); overflow: hidden; | |
| @@ -1,6 +1,8 @@ | |||||
| 1 | @layer components { | 1 | @layer components { | ||
| 2 | /* merge requests: the review page. Hue only where it carries a verdict. */ | 2 | /* merge requests: the review page. Hue only where it carries a verdict. */ | ||
| 3 | .mr-h .show-kv { margin-top: 0; } | 3 | .mr-h .show-kv { margin-top: 0; } | ||
| 4 | + | #merge-menu .htmx-indicator { display: none; } | |||
| 5 | + | #merge-menu .htmx-request .htmx-indicator { display: inline; } | |||
| 4 | /* one card: the header (title, facts) a shade lighter, the description below */ | 6 | /* one card: the header (title, facts) a shade lighter, the description below */ | ||
| 5 | .mr-card { | 7 | .mr-card { | ||
| 6 | background: var(--bg-surface); border: 1px solid var(--border); border-radius: var(--r); overflow: hidden; | 8 | background: var(--bg-surface); border: 1px solid var(--border); border-radius: var(--r); overflow: hidden; | ||
M internal/adapters/web/static/js/mr.js +3 -3
| @@ -293,8 +293,8 @@ window.ermiteMR = (() => { | |||
| 293 | 293 | let observer; | |
| 294 | 294 | const summary = $('#mr-ck'); | |
| 295 | 295 | if (summary) { | |
| 296 | - | // The live stream replaces #mr-ck as a whole: watch only the children of | |
| 297 | - | // its parent, and write the label only when it changes. hx-live rewrites | |
| 296 | + | // Both #mr-ck and #mr-actions can be replaced: observe the stable bar, | |
| 297 | + | // and write the label only when it changes. hx-live rewrites | |
| 298 | 298 | // other text in the same bar on every mutation of the document; a write | |
| 299 | 299 | // here on each of them would wake hx-live again, a loop that never yields. | |
| 300 | 300 | const sync = () => { | |
| @@ -303,7 +303,7 @@ window.ermiteMR = (() => { | |||
| 303 | 303 | if (label && label.textContent !== text) label.textContent = text; | |
| 304 | 304 | }; | |
| 305 | 305 | observer = new MutationObserver(sync); | |
| 306 | - | observer.observe(summary.parentElement, { childList: true }); | |
| 306 | + | observer.observe(summary.closest('.mr-bar'), { childList: true, subtree: true }); | |
| 307 | 307 | sync(); | |
| 308 | 308 | } | |
| 309 | 309 | show(initial); | |
| @@ -293,8 +293,8 @@ window.ermiteMR = (() => { | |||||
| 293 | let observer; | 293 | let observer; | ||
| 294 | const summary = $('#mr-ck'); | 294 | const summary = $('#mr-ck'); | ||
| 295 | if (summary) { | 295 | if (summary) { | ||
| 296 | - | // The live stream replaces #mr-ck as a whole: watch only the children of | 296 | + | // Both #mr-ck and #mr-actions can be replaced: observe the stable bar, |
| 297 | - | // its parent, and write the label only when it changes. hx-live rewrites | 297 | + | // and write the label only when it changes. hx-live rewrites |
| 298 | // other text in the same bar on every mutation of the document; a write | 298 | // other text in the same bar on every mutation of the document; a write | ||
| 299 | // here on each of them would wake hx-live again, a loop that never yields. | 299 | // here on each of them would wake hx-live again, a loop that never yields. | ||
| 300 | const sync = () => { | 300 | const sync = () => { | ||
| @@ -303,7 +303,7 @@ window.ermiteMR = (() => { | |||||
| 303 | if (label && label.textContent !== text) label.textContent = text; | 303 | if (label && label.textContent !== text) label.textContent = text; | ||
| 304 | }; | 304 | }; | ||
| 305 | observer = new MutationObserver(sync); | 305 | observer = new MutationObserver(sync); | ||
| 306 | - | observer.observe(summary.parentElement, { childList: true }); | 306 | + | observer.observe(summary.closest('.mr-bar'), { childList: true, subtree: true }); |
| 307 | sync(); | 307 | sync(); | ||
| 308 | } | 308 | } | ||
| 309 | show(initial); | 309 | show(initial); | ||