Skip to content

Commit 0dc4efd

Browse files
pedrogaudenciopieertaoeffect
authored
Change request revision rounds (#154)
* editor: implement fork-on-edit workflow for article editing * set fork-related context data * show submit button based on permissions * add ForkAndEdit field to EditRepoFileForm * modify EditFilePost to handle fork creation and commit to fork * reuse existing fork if user already has one * editor: prevent editing fork when user owns repo for same subject * query repos by owner and subject * check for existing user repo for subject * disable edit/fork buttons when user already has a repo for the subject * show message in edit view explaining why editing is disabled * editor: remove unused form parameter from handleForkAndEdit * editor: fix getUniqueRepositoryName bugs and optimize * fix wrong error condition (returned on any error, not just 'not found') * fix double increment bug (i++ in loop body + loop increment) * optimise from O(n) queries to single query with hash set lookup O(1) * add proper error logging * fork: parallelize permission queries * run subject ownership check and fork detection concurrently * fork: add ErrUserOwnsSubjectRepo error type * add error for subject ownership violations * validate subject ownership before forking * editor: consolidate fork-on-edit permission logic * fix POST handler to validate subject ownership * templates: update article editing logic * migrations: add composite indexes for fork-on-edit optimization * create IDX_repository_owner_subject and IDX_repository_owner_fork * optimizes GetRepositoryByOwnerIDAndSubjectID() and GetForkedRepo() queries * editor: improve fork-on-edit error messages in templates * replace 'Cannot Edit' button with 'Edit Your Article' action link * add translation keys for fork-on-edit error messages * link users to their existing article when subject ownership blocks editing * i18n: replace hardcoded strings with translation variables in article editor * templates: add translation keys and simplify logic * simplify fork_and_edit hidden field to only use .NeedsFork * extract repeated nil-guard pattern into $subjectName template variable * article: add confirmation modal to fork article button * article: add tooltip to Fork button * tests: add e2e tests for fork article * add Playwright tests for modal appearance, cancel actions, and tooltip * test modal shows correct header, content, and button text * verify cancel button and Escape key close modal without action * verify fork button has tooltip with confirmation message * tests: add fork-on-edit permission e2e tests * add repository owner tests for Submit Changes button behavior * add non-owner tests for Fork button and disabled submit button * add unauthenticated user test for sign-in button * tests: add edge case and accessibility tests for fork article modal * accessibility tests verify Enter opens modal, Tab navigates buttons, and buttons have proper accessible text content * unique name tests verify suffix generation when base name is taken * editor: fix fork-and-edit workflow for non-owners * use RepoOperationsLink instead of RepoLink in form action url * bypass fork_and_edit in CanWriteToBranch middleware to allow non-owners to reach the EditFilePost handler * skip NeedFork workflow when ForkAndEdit is true to prevent conflicting fork creation attempts * tests: fork-and-edit workflow * CheckForkOnEditPermissions for owner/non-owner/unauthenticated * CanWriteToBranch middleware bypass with fork_and_edit=true * existing fork detection using fixture data (repo11/repo10) * subject ownership blocking prevents duplicate forks * form action URL uses RepoOperationsLink not RepoLink * tests: add database migration tests for v326 and v327 * slug generation migration covering basic slugs, mixed-case deduplication, special characters, unique constraint verification, and empty table handling * index creation, idempotency, and index type verification * api: add tests for fork with subject conflict and fix error handling order * fork API behavior when user already owns a repository for the same subject * fork blocked by subject ownership (403), fork succeeds for user without subject repo (202), fork blocked even with custom name (403) * fix error handling order in CreateFork * permissions: restrict fork_and_edit bypass to _edit and _new actions only * tests: add fork-on-edit permission tests * fork: replace type assertion with wrapped error support * templates: document fallback logic * repo: return errors from GetForkedRepo * update GetForkedRepo signature to return (*Repository, error) * update all callers to handle the new error return value * fixes silent error swallowing * linting: fix issues * fix modal promise resolution pattern * add PathEscapeSegments to article template URL * update test comment to list all removed special characters * tests: use slices instead of forloop * tests: fix ESLint and TypeScript errors * replace page.waitForSelector() with expect(locator).toBeAttached() to satisfy playwright/no-wait-for-selector rule * fix accessibility test to use proper Playwright assertions instead of treating Locators as strings * tests: fix toastui-editor selector * toast UI Editor creates two .toastui-editor elements (md-mode and ww-mode) * use .first() to satisfy Playwright's strict mode requirement * fork-on-edit: address code review findings * add maxUniqueNameAttempts constant to replace magic number 1000 * use t.Cleanup with nil check instead of defer for safer test cleanup * remove unused data-existing-fork attribute from article template * add comment explaining that subject ownership check takes precedence * editor: implement submit-change-request for article contributions * create unique branch, commit changes, and create CR * add /article/{username}/{subjectname}/pulls/{index} route group * mirror standard repo PR routes with RepoAssignmentByOwnerAndSubject middleware * permission: allow submit_change_request to bypass branch write check * update middleware to bypass permission check for submit_change_request=true * enable non-owners to submit change requests * editor: fix log message format * editor: validate repository allows pull requests * editor: add content validation * tests: update fork-article-modal tests to use new button ID * tests: add submit-change-request workflow tests * add integration tests for submit-change-request * add e2e tests for submit-change-request UI * add permission bypass tests for submit_change_request * tests cover CR creation, branch naming, error cases, security bypass prevention * editor: add InternalPush option for submit-change-request workflow * add InternalPush field to explicitly skip pre-receive hooks * add PushWithOptions method to TemporaryUploadRepository for internal push * fix test to correctly verify middleware checks form values * tests: fix E2E tests for submit-change-request * add force clicks and scroll-into-view for mobile browser compatibility * increase modal visibility timeouts from 5s to 10s for mobile devices * add 300ms wait for modal animation before interacting with buttons * fix fork-article-modal test to expect correct button combination * tests: fix linting in submit-change-request * pull: restrict merge operations to repository owners only * add ownership validation in MergePullRequest before any merge processing * update template to hide merge UI for non-owners * pull: enforce squash-only merge strategy * remove all merge styles except 'squash' from mergeStyles array * update template condition to only check AllowSquash setting * hide merge style dropdown when only one option is available * editor: add custom title and description for Change Request * editor: add modal UI for Change Request title and description * tests: suppress linting warnings in submit-change-request * fix: use distinct button IDs and correct E2E test selectors - Fix E2E tests to use #pre-submit-changes-button instead of #submit-changes-button[data-submit-change-request='true'] - Fix E2E tests to use #submit-change-request-modal instead of .ui.g-modal-confirm.modal for the Submit Change Request modal - Template already uses distinct IDs: - #submit-changes-button for owner's direct submit - #fork-article-button for fork-and-edit - #pre-submit-changes-button for submit change request Addresses Copilot review feedback about button ID ambiguity. * fix: use translation key for content required error message - Add new translation key 'repo.editor.content_required' - Replace hardcoded 'content is required' string in editor.go - Improves internationalization support Addresses Copilot review feedback about hardcoded error string. * fix: add explicit authentication check in handleSubmitChangeRequest - Add nil check for ctx.Doer at the start of handleSubmitChangeRequest - Defense-in-depth: prevents potential nil pointer panics - Middleware should already handle auth, but explicit check is safer Addresses Copilot review feedback about missing permission validation. * fix: validate mutually exclusive fork_and_edit and submit_change_request flags - Add validation to reject requests where both flags are true - Prevents unexpected behavior if JavaScript fails to set flags correctly - Security defense-in-depth for the middleware OR condition Addresses Copilot review feedback about mutually exclusive form flags. * docs: fix integration test comment about middleware behavior - Clarify that FormValue() checks both query params AND form data - Accurately describes middleware behavior Addresses Copilot review feedback about test comment mismatch. * tests: fix selector for Submit Change Request button * update selector from #submit-changes-button to #pre-submit-changes-button * editor: validate empty content in submit change request * editor: add CR title length validation * enforce 255-character limit on CR titles * editor: cleanup orphaned branch on CR creation failure * tests: branch cleanup on CR failure * verify orphaned branches are cleaned up when NewPullRequest() fails * tests: whitespace-only content rejection * linting: fix issues * Remove side bar from the pull request * tests: add concurrent branch collision test for submit-change-request * verify unique branch name generation when multiple users submit change requests concurrently * linting: fix issues * Fix linting * actions: fix empty Ref warning for workflow_run events * set Ref from run.Ref in WorkflowRunStatusUpdate notifier to avoid warning * increase test timeout for PR synchronize event from 1s to 10s since AddTestPullRequestTask runs asynchronously in a goroutine * templates: move issue view changes to custom overrides * revert base templates to upstream state * routers: fix nil pointer dereference * return ServerError when LoadBaseRepo fails instead of just logging * prevent nil pointer dereference when accessing pull.BaseRepo fields * tests: fix submit-change-request concurrent test assumptions * update test to match actualimplementation which creates same-repo PRs, not forks * remove incorrect fork lookup logic that expected user4/repo1 and user5/repo1 * query PRs from target repository and verify HeadRepoID == BaseRepoID == repo.ID * update redirect URL assertions to expect target repo path, not user forks * pull: allow non-collaborator CR creation for submit-change-request workflow * add AllowNonCollaborator flag to NewPullRequestOptions struct * bypass collaborator check when flag is set (caller must verify authorization) * set flag in handleSubmitChangeRequest where middleware already verified access * editor: fix submit-change-request creating PRs in wrong repository * skip NeedFork workflow when SubmitChangeRequest is true * handleSubmitChangeRequest function creates branches directly in the target repository without forking * fix test expectations to use article URL format when repository has a subject * Fix NewEndpointAllowsSubmitChangeRequest test to use README.md path to avoid redirect when NeedFork is true * locale: update change request copy * editor: fix orphaned branches when OpenRepository or GetCompareInfo fails * Add branch cleanup to OpenRepository and GetCompareInfo error path in handleSubmitChangeRequest * Prevent orphaned branches from accumulating when these specific errors occur * editor: fix UTF-8 corruption in PR title truncation * replace byte-level slicing with util.TruncateRunes() to prevent splitting multi-byte UTF-8 characters at byte position 255 * The 255 limit is for characters (runes), not bytes * fork-on-edit: allow change requests from users with existing forks * fix permissions to distinguish between user's fork of a repo vs independent article for same subject * add logic to submit change request button when user has existing fork * add validation to prevent submitting change requests to own repository * add CanSubmitChangeRequest permission flag * editor: add permission validation for submit change request * check IsRepoOwner, BlockedBySubject and CanSubmitChangeRequest and return appropriate error * add defense-in-depth validation to prevent API bypass * locale: standardize key * templates: use conditionally show submit button * tests: add unit and integration tests for fork-on-edit permissions * add unit tests covering repo owner, non-owner, fork owner, and anonymous user scenarios * add integration tests validating CR submission permissions for fork owners * editor: fix PushOutOfDate error * check both database AND git repository for branch existence in getUniquePatchBranchName() * prevents non-fast-forward push errors when branches exist in git but not in the database * locale: fix typos * editor: remove redundant cleanup * tests: fix hardcoded PR index * extract PR index dynamically from redirect URL instead of assuming index 1 * use require.NoError/NotNil for proper error handling on PR lookup * editor: extract duplicate branch cleanup logic to helper function * add cleanupOrphanedBranch helper to reduce duplication * editor: add specific error message for mutually exclusive workflow flags * editor: add PR description length limit for change requests * Update tooltip color and message * Update message after fork * Fix table ui * Change WYSIWYG * Update owner review header * Update button style * Update copy and design * Style split view * Update Submit review into popup box * Customize combo-markdown-editor * routers: fix comment form action url and add missing article-namespace update routes * fix template to use /pulls/ instead of /issues/ in change request urls * register article-namespace view routes for change request info, attachments, content-history * register article-namespace update routes for comments, reactions, title, content, and all other change request update operations * register article-namespace comment edit/delete and pull-specific review routes * routers: remove redundant middleware * remove redundant RepoMustNotBeArchived() on inner article pulls group * Update PR comment form buttons and styling * Fix overlay on the popup when a comment box is open the markdown editor * attempt to fix the failing tests * feat: Localize the pull request review modal and diff headers, and refactor the review modal's display logic. * refactor: rename new_review.tmpl to review_submit.tmpl * refactor: Introduce and apply CSS variables for standardized color and font-weight styling. * Fix test * Fix split view * repo: fix missing middleware * fixes missing middleware on article route * added defensive type assertion Fixes /issues/151 * routers: add register pull request helper * templates: fix review button condition * repo: add type assertion fix * repo: add edit tab on request changes * repo: add submit edit in change request * pull: sync branches to db before editing Change Request files * branches created via InternalPush bypass post-receive hooks and are never synced to the branch table * ChangeRepoFiles checks the db for branch existence and fails for these unsynced branches * add SyncRepoBranches call before ChangeRepoFiles in SubmitPullEditPost to ensure the head branch is registered in the db * pull: emit push comment after commit * pull: gate Files tab edit button on active rejection, not write permission * replace GetUserRepoPermission/CanWrite check with poster + reject-review check * show 'Edit this file' only when viewer is the PR poster, the PR is open, and there is a non-dismissed review * pull: fix edit tab order and copy * pull: fix stale refs/pull/N/head after SubmitPullEditPost * Fix PR review feedback: navbar pill active state and review button auth check - Make navbar-pill-active conditional on PageIsPullList so it's only highlighted on PR-related pages (fixes always-active pill bug) - Add $.SignedUserID check to review submit button condition to prevent showing it to unauthenticated users - Fix split/unified view toggle (replace broken JS onclick with server-side rendered <a> tag) - Add mobile default to unified view - Add CSS overrides for conversation-holder comments and PR diff styling * Fix test: render review button for all PR file views Remove .IsShowingAllCommits from the outer condition in box.tmpl since review_submit.tmpl already handles the disabled state internally when not showing all commits. This matches the master behavior where the review button is always rendered for signed-in users on PR file pages. Fixes TestPullDiff_SingleCommitPRDiff and TestPullDiff_CommitRangePRDiff which expect .js-btn-review to exist with a disabled class. * fix: add missing data-fork-confirm-body2 attribute in article template The JavaScript in article-editor.ts reads data-fork-confirm-body2 to display a second paragraph in the fork confirmation modal, but the template was missing this attribute. The locale key already exists. Fixes AI Review Issue #1. * fix: remove unused review title field from review modal The backend SubmitReviewForm struct has no Title field, so the title input was silently discarded. This removes it to avoid misleading users. Fixes AI Review Issue #2. * cleanup: remove commented-out CSS and orphaned media query Remove large blocks of commented-out CSS for .issue-content-* classes and .labels-list, plus the orphaned media query that referenced deleted classes. Git history preserves these styles if needed. Fixes AI Review Issues #3 and #9. * fix: replace inline onclick handlers with event listeners Move inline onclick handlers to addEventListener calls for CSP compatibility. Affects the diff info banner dismiss button in box.tmpl and the PR back link in view_title.tmpl. Fixes AI Review Issue #4. * fix: add Escape key handler for review modal (accessibility) The review modal could only be closed by clicking the X button or the overlay. Add a keydown listener so pressing Escape also closes it. Fixes AI Review Issue #5. * i18n: replace hardcoded English strings with locale lookups Replace 'Post immediately', 'Save draft', and 'Click a paragraph to add a comment.' with ctx.Locale.Tr calls. Add corresponding keys to the custom locale file. Fixes AI Review Issue #6. * fix: prevent mobile redirect loop on PR files page Add sessionStorage guard so the automatic redirect from split to unified view on mobile only fires once per session. Previously, if a user switched back to split view, they'd be immediately redirected again. Fixes AI Review Issue #7. * routers: add RepoMustNotBeArchived to PR edit POST route * prevents editing pull requests in archived repositories * routers: add CommitID check to SubmitPullEditPost * prevents PR edits when head has moved past the 'Request Changes' review * routers: add user feedback for post-edit failures * show flash warning when PushToBaseRepo fails after PR edit * show flash warning when MarkReviewsAsStale fails after PR edit * templates: remove unused data attributes from PR edit template * routers: add empty content validation to PR edit handler * pulls: set NumCommits=0 in ViewPullEdit to fix <nil> pill * ViewPullEdit renders the shared tab_menu template which displays {{.NumCommits}} in the Changes tab badge * routers: extract preparePullEditTabVisibility, fix missing Edit tab on Files/Commits tabs * HasChangesRequested was only set by ViewIssue and ViewPullEdit; both viewPullFiles and ViewPullCommits render tab_menu.tmpl but never set the flag, so the Edit tab vanished on every click to the Files or Commits tab * new preparePullEditTabVisibility helper sets the flag and returns the ReviewList for reuse, removing the duplicate FindReviews query that existed inside the CanEditFile block in viewPullFiles * repo: populate PR header context in ViewPullEdit * call preparePullViewPullInfo to set HeadTarget, BaseTarget, branch links, and real NumCommits (was hardcoded 0) * repo: gate ViewPullEdit on CommitID like SubmitPullEditPost * add CommitID-matching loop in ViewPullEdit matching the existing gate in SubmitPullEditPost and the 'Edit' button in viewPullFiles * add TestViewPullEditCommitIDGate covering the no-review, matching, and stale-review cases end-to-end * repo: fix tree_path traversal in SubmitPullEditPost * resolve article README path server-side from head branch commit instead of trusting the client-controlled 'tree_path' form field * malicious PR poster can no longer overwrite arbitrary files by tampering the hidden input via browser DevTools * pulls: hide Edit tab for stale request-changes reviews * pull: fail SubmitPullEditPost on PushToBaseRepo error * replace non-fatal Flash.Warning with ctx.JSONError + return so a ref-update failure surfaces as a real error to the user * prevents the user from being redirected to a PR page whose refs/pull/N/head is stale after a failed push * pull: add comment explaining push flow * pull: make PushToBaseRepo failure a non-fatal flash warning * routers: trim whitespace from commit_summary before checking if empty * trim commit_summary form field before evaluating if it's empty * add test to verify whitespace-only commit_summary uses default message * routers: improve error handling in SubmitPullEditPost for user-facing errors * replace blanket ServerError with proper error type discrimination * handle stale commit ID (ErrCommitIDDoesNotMatch) as JSONError with user-friendly message * handle concurrent push (ErrPushOutOfDate) as JSONError with same message * handle missing file (ErrRepoFileDoesNotExist, ErrNotExist) as JSONError * add test to verify stale commit ID returns JSONError instead of HTTP 500 * routers: only fall back to README.md on not-exist errors in pull edit handlers * ViewPullEdit: surface I/O / corruption errors immediately instead of masking them by falling through to README.md * SubmitPullEditPost: same guard in the candidate-probe loop so a real read error on @README.md cannot cause a silent write to the wrong path * tests: fix TestSubmitChangeRequestWhitespaceOnlyCommitSummary using wrong ID type * editor: localize submit-change-request commit message * replace hard-coded default with new repo.editor.update_article key * add server-side guard: reject an empty computed commit message with repo.editor.commit_message_required * tests: add missing commit_choice field in TestSubmitPullEditPostStaleCommitID * add to the owner's edit form to satisfy server-side validation that rejects empty commit_choice with HTTP 400 * tests: use API endpoint for concurrent edit in TestSubmitPullEditPostStaleCommitID * the _edit endpoint's middleware chain couldn't resolve the PR head branch created by submit-change-request, causing HTTP 400 * API endpoint is simpler and more reliable for programmatic file updates * tests: fix tests * replace GetCommit(branchName) with GetBranchCommit() (correct API) * fix stale-review test: expect 404 from CommitID gate, add fresh review step before testing stale POST * editor: sync branch to DB after InternalPush in submit-change-request * InternalPush bypasses post-receive hooks, leaving the branch only in git but not in the database branch table * subsequent API calls that check branch existence via the DB (e.g. ChangeRepoFiles) fail with 'branch does not exist' (HTTP 404) * call SyncBranchesToDB right after creating the branch to ensure consistency between git and the database * editor: hard-delete orphaned branch DB record on cleanup * cleanupOrphanedBranch now removes the soft-deleted DB record after DeleteBranch, since the branch was never meant to exist * fixes TestSubmitChangeRequestPRCreationFailureCleanup which expected no trace of the orphaned branch after PR creation failure --------- Co-authored-by: Pierre Schweiger <schweiger.pierre@gmail.com> Co-authored-by: Greg Slepak <contact@taoeffect.com>
1 parent 1045b44 commit 0dc4efd

File tree

9 files changed

+1075
-15
lines changed

9 files changed

+1075
-15
lines changed

custom/options/locale/locale_en-US.ini

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,8 +1120,10 @@ editor.change_request_description_label = Add a description
11201120
editor.change_request_description_placeholder = Explain what you changed and why
11211121
editor.change_request_title_required = Please enter a title for your change request
11221122
editor.content_required = Content is required
1123+
editor.commit_message_required = Commit message is required
11231124
editor.cannot_submit_change_request_to_own_repo = You cannot submit a change request to your own repository. Use direct edit instead.
11241125
editor.cannot_create_branch = Failed to submit your changes.
1126+
editor.file_not_found = The article file could not be found.
11251127
editor.no_change_request_permission = You do not have permission to submit change requests to this repository.
11261128
editor.editing_unavailable = Editing is currently unavailable.
11271129
editor.sign_in_to_edit = Sign in to Edit
@@ -1409,7 +1411,7 @@ editor.new_file = New File
14091411
editor.create_article = Create Article
14101412
editor.article_creation = Article creation
14111413
editor.upload_file = Upload File
1412-
editor.edit_file = Edit File
1414+
editor.edit_file = Edit
14131415
editor.preview_changes = Preview Changes
14141416
editor.cannot_edit_lfs_files = LFS files cannot be edited in the web interface.
14151417
editor.cannot_edit_too_large_file = The file is too large to be edited.
@@ -1431,6 +1433,7 @@ editor.commit_changes = Commit Changes
14311433
editor.add_tmpl = Add '{filename}'
14321434
editor.add = Add %s
14331435
editor.update = Update %s
1436+
editor.update_article = Update article
14341437
editor.delete = Delete %s
14351438
editor.patch = Apply Patch
14361439
editor.patching = Patching:
@@ -1829,8 +1832,8 @@ issues.due_date = Due Date
18291832
issues.invalid_due_date_format = "Due date format must be 'yyyy-mm-dd'."
18301833
issues.error_modifying_due_date = "Failed to modify the due date."
18311834
issues.error_removing_due_date = "Failed to remove the due date."
1832-
issues.push_commit_1 = "added %d commit %s"
1833-
issues.push_commits_n = "added %d commits %s"
1835+
issues.push_commit_1 = "added %d change %s"
1836+
issues.push_commits_n = "added %d changes %s"
18341837
issues.force_push_codes = `force-pushed %[1]s from <a class="ui sha" href="%[3]s"><code>%[2]s</code></a> to <a class="ui sha" href="%[5]s"><code>%[4]s</code></a> %[6]s`
18351838
issues.force_push_compare = Compare
18361839
issues.due_date_form = "yyyy-mm-dd"
@@ -1928,6 +1931,8 @@ pulls.new = New Change Request
19281931
pulls.new.blocked_user = Cannot create change request because you are blocked by the repository owner.
19291932
pulls.new.must_collaborator = You must be a collaborator to create change request.
19301933
pulls.edit.already_changed = Unable to save changes to the change request. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes.
1934+
pulls.edit.ref_update_failed = Your changes were saved, but the change request display could not be updated. Reload the page to see the latest diff and commit count.
1935+
pulls.edit.reviews_stale_failed = Your changes were saved, but the existing reviews could not be marked as stale. They may not visually update until the next review activity.
19311936
pulls.view = View Change Request
19321937
pulls.compare_changes = New Change Request
19331938
pulls.allow_edits_from_maintainers = Allow edits from maintainers
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
{{template "base/head" .}}
2+
<div role="main" aria-label="{{.Title}}" class="page-content repository view issue pull">
3+
{{template "repo/header" .}}
4+
<div class="ui container">
5+
{{template "repo/issue/view_title" .}}
6+
{{template "repo/pulls/tab_menu" .}}
7+
<div class="ui bottom attached">
8+
<form class="ui edit form" id="article-edit-form" method="post" action="{{.Issue.Link}}/edit">
9+
{{.CsrfTokenHtml}}
10+
<input type="hidden" name="last_commit" value="{{.LastCommitID}}">
11+
<input type="hidden" name="tree_path" value="{{.ReadmeTreePath}}">
12+
<input type="hidden" name="commit_choice" value="direct">
13+
14+
<div class="field">
15+
<div class="tw-p-0">
16+
<textarea id="edit_area" name="content" class="tw-hidden" data-id="repo-{{.Repository.Name}}-{{.ReadmeTreePath}}">{{.FileContent}}</textarea>
17+
<div class="editor-loading is-loading"></div>
18+
<div id="toast-editor-container" style="min-height: 500px;"></div>
19+
</div>
20+
</div>
21+
22+
{{/* Commit form */}}
23+
<div class="tw-mt-4 tw-p-4" style="border-top: 1px solid var(--color-secondary);">
24+
<div class="ui form">
25+
<div class="field">
26+
<label for="commit_summary">{{ctx.Locale.Tr "repo.diff.review.add_title"}}</label>
27+
<input type="text" id="commit_summary" name="commit_summary" maxlength="100">
28+
</div>
29+
<div class="field">
30+
<label for="review_comment">{{ctx.Locale.Tr "repo.diff.review.summary"}}</label>
31+
<textarea id="review_comment" name="review_comment" rows="4" placeholder="{{ctx.Locale.Tr "repo.diff.review.placeholder"}}"></textarea>
32+
</div>
33+
<div class="tw-mt-4 tw-flex tw-justify-end tw-gap-2">
34+
<a class="ui button" href="{{.Issue.Link}}">
35+
{{svg "octicon-x" 16 "tw-mr-1"}}
36+
{{ctx.Locale.Tr "cancel"}}
37+
</a>
38+
<button type="button" class="ui primary button" id="submit-changes-button">
39+
{{svg "octicon-check" 16 "tw-mr-1"}}
40+
{{ctx.Locale.Tr "repo.editor.submit_changes"}}
41+
</button>
42+
</div>
43+
</div>
44+
</div>
45+
</form>
46+
</div>
47+
</div>
48+
</div>
49+
{{template "base/footer" .}}
50+

custom/templates/repo/pulls/tab_menu.tmpl

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@
55
{{template "shared/misc/tabtitle" (ctx.Locale.Tr "repo.pulls.tab_conversation")}}
66
<span class="ui small label">{{.Issue.NumComments}}</span>
77
</a>
8+
{{if and .IsIssuePoster (not .Issue.IsClosed) (not .Issue.PullRequest.HasMerged) .HasChangesRequested}}
9+
<a class="item {{if .PageIsPullEdit}}active{{end}}" href="{{.Issue.Link}}/edit">
10+
{{svg "octicon-pencil"}}
11+
{{template "shared/misc/tabtitle" (ctx.Locale.Tr "repo.editor.edit_file")}}
12+
</a>
13+
{{end}}
814
<a class="item {{if .PageIsPullFiles}}active{{end}}" href="{{.Issue.Link}}/files">
915
{{svg "octicon-diff"}}
1016
{{template "shared/misc/tabtitle" (ctx.Locale.Tr "repo.pulls.tab_files")}}
11-
<span class="ui small label pill-label">{{if .NumFiles}}{{.NumFiles}}{{else}}-{{end}}</span>
17+
<span class="ui small label pill-label">{{.NumCommits}}</span>
1218
</a>
1319
</div>
1420
<div class="ui tabs divider"></div>
1521
</div>
22+

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,7 @@ editor.commit_changes = Commit Changes
13641364
editor.add_tmpl = Add '{filename}'
13651365
editor.add = Add %s
13661366
editor.update = Update %s
1367+
editor.update_article = Update article
13671368
editor.delete = Delete %s
13681369
editor.patch = Apply Patch
13691370
editor.patching = Patching:

routers/web/repo/editor.go

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,8 @@ func handleForkAndEdit(ctx *context.Context) *repo_model.Repository {
608608

609609
// cleanupOrphanedBranch attempts to delete a branch that was created but is no longer needed
610610
// (e.g., when PR creation fails after the branch was already created).
611+
// It performs both a soft-delete (via DeleteBranch) and a hard-delete of the DB
612+
// record, since the branch was never meant to exist and should leave no trace.
611613
// It logs any errors but does not propagate them to the caller.
612614
func cleanupOrphanedBranch(ctx *context.Context, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) {
613615
if gitRepo == nil {
@@ -622,6 +624,19 @@ func cleanupOrphanedBranch(ctx *context.Context, repo *repo_model.Repository, gi
622624
SkipPermissionCheck: true,
623625
}); err != nil {
624626
log.Error("cleanupOrphanedBranch: failed to cleanup branch %s: %v", branchName, err)
627+
return
628+
}
629+
630+
// Hard-delete the soft-deleted DB record so the orphaned branch leaves no
631+
// trace. DeleteBranch only marks the record as deleted (is_deleted=true);
632+
// without this step the branch would still appear in unfiltered queries.
633+
branch, err := git_model.GetBranch(ctx, repo.ID, branchName)
634+
if err != nil {
635+
log.Error("cleanupOrphanedBranch: failed to get branch record for %s: %v", branchName, err)
636+
return
637+
}
638+
if err := git_model.RemoveDeletedBranchByID(ctx, repo.ID, branch.ID); err != nil {
639+
log.Error("cleanupOrphanedBranch: failed to hard-delete branch record for %s: %v", branchName, err)
625640
}
626641
}
627642

@@ -687,7 +702,12 @@ func handleSubmitChangeRequest(ctx *context.Context, form *forms.EditRepoFileFor
687702
// The ChangeRepoFiles function will create the new branch from the default branch
688703
// We use InternalPush to skip pre-receive hooks since this is a programmatic operation
689704
// where we've already verified the user can submit change requests (via middleware)
690-
defaultCommitMessage := ctx.Locale.TrString("repo.editor.update", form.TreePath)
705+
defaultCommitMessage := ctx.Locale.TrString("repo.editor.update_article")
706+
commitMessage := parsed.GetCommitMessage(defaultCommitMessage)
707+
if strings.TrimSpace(commitMessage) == "" {
708+
ctx.JSONError(ctx.Tr("repo.editor.commit_message_required"))
709+
return nil
710+
}
691711
_, err = files_service.ChangeRepoFiles(ctx, targetRepo, ctx.Doer, &files_service.ChangeRepoFilesOptions{
692712
// Use an empty LastCommitID so ChangeRepoFiles bases the new commit on the current
693713
// HEAD of OldBranch. In this workflow we always create a new branch (NewBranch != OldBranch),
@@ -697,7 +717,7 @@ func handleSubmitChangeRequest(ctx *context.Context, form *forms.EditRepoFileFor
697717
LastCommitID: "",
698718
OldBranch: targetRepo.DefaultBranch,
699719
NewBranch: branchName,
700-
Message: parsed.GetCommitMessage(defaultCommitMessage),
720+
Message: commitMessage,
701721
Files: []*files_service.ChangeRepoFile{
702722
{
703723
Operation: "update",
@@ -727,6 +747,26 @@ func handleSubmitChangeRequest(ctx *context.Context, form *forms.EditRepoFileFor
727747
}
728748
defer gitRepo.Close()
729749

750+
// Sync the newly created branch to the database. The InternalPush above
751+
// bypasses post-receive hooks, so the branch exists only in git at this
752+
// point. Without this sync, any subsequent operation that checks branch
753+
// existence via the DB (e.g. ChangeRepoFiles from the API) would fail
754+
// with "branch does not exist".
755+
newCommitID, err := gitRepo.GetBranchCommitID(branchName)
756+
if err != nil {
757+
log.Error("handleSubmitChangeRequest: failed to get branch commit ID: %v", err)
758+
cleanupOrphanedBranch(ctx, targetRepo, gitRepo, branchName)
759+
ctx.ServerError("GetBranchCommitID", err)
760+
return nil
761+
}
762+
if err := repo_service.SyncBranchesToDB(ctx, targetRepo.ID, ctx.Doer.ID,
763+
[]string{branchName}, []string{newCommitID}, gitRepo.GetCommit); err != nil {
764+
log.Error("handleSubmitChangeRequest: failed to sync branch to DB: %v", err)
765+
cleanupOrphanedBranch(ctx, targetRepo, gitRepo, branchName)
766+
ctx.ServerError("SyncBranchesToDB", err)
767+
return nil
768+
}
769+
730770
// Same-repo CR: both head and base are in the target repository
731771
compareInfo, err := pull_service.GetCompareInfo(ctx, targetRepo, targetRepo, gitRepo,
732772
git.BranchPrefix+targetRepo.DefaultBranch, git.BranchPrefix+branchName, false, false)

routers/web/repo/issue_view.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,12 @@ func ViewIssue(ctx *context.Context) {
417417
ctx.Data["LockReasons"] = setting.Repository.Issue.LockReasons
418418
ctx.Data["RefEndName"] = git.RefName(issue.Ref).ShortName()
419419

420+
// Set HasChangesRequested for the Edit tab in the PR tab menu.
421+
preparePullEditTabVisibility(ctx, issue)
422+
if ctx.Written() {
423+
return
424+
}
425+
420426
tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID)
421427
if err != nil {
422428
ctx.ServerError("GetTagNamesByRepoID", err)

0 commit comments

Comments
 (0)