Change request: enable submitting article edits #101

pedrogaudencio · 2025-12-31T13:06:54Z

* create unique branch, commit changes, and create CR * add /article/{username}/{subjectname}/pulls/{index} route group * mirror standard repo PR routes with RepoAssignmentByOwnerAndSubject middleware

* update middleware to bypass permission check for submit_change_request=true * enable non-owners to submit change requests

* 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

Copilot

Pull request overview

This pull request implements the "submit change request" functionality for article contributions, enabling non-owners to propose changes without creating their own fork. The feature creates a branch in the target repository, commits changes, and automatically creates a pull request back to the default branch.

Key changes:

Adds submit change request workflow that creates in-repo PRs instead of fork-based PRs
Implements article pull request routes mirroring standard repository PR routes
Updates permission middleware to bypass write checks for submit change request actions

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
web_src/js/features/article-editor.ts	Separates fork button from submit change request button handlers and adds modal confirmation
tests/integration/submit_change_request_test.go	Adds integration tests for submit change request workflow and security bypass validation
tests/integration/repo_fork_edit_test.go	Extends fork-and-edit tests to cover submit change request middleware bypass
tests/e2e/submit-change-request.test.e2e.ts	Adds E2E tests for button visibility, modals, and accessibility
tests/e2e/fork-article-modal.test.e2e.ts	Updates button selectors from 'submit-changes-button' to 'fork-article-button'
services/forms/repo_form_editor.go	Adds SubmitChangeRequest boolean field to EditRepoFileForm
services/context/permission.go	Updates CanWriteToBranch middleware to bypass for submit_change_request flag
routers/web/web.go	Registers article-based pull request routes using RepoAssignmentByOwnerAndSubject
routers/web/repo/editor.go	Implements handleSubmitChangeRequest function to create branch and PR
options/locale/locale_en-US.ini	Adds locale strings for submit changes UI and error messages
custom/templates/shared/repo/edit.tmpl	Adds hidden form field for submit_change_request flag
custom/templates/shared/repo/article.tmpl	Adds Submit Change Request button alongside Fork Article button
custom/options/locale/locale_en-US.ini	Adds custom locale strings for submit change request feature

Comments suppressed due to low confidence (1)

web_src/js/features/article-editor.ts:120

The event listener is attached to any button with ID 'submit-changes-button', regardless of whether it's the owner's direct submit button or the non-owner's submit change request button. This creates ambiguity because both buttons share the same ID in the template. The code checks for the 'data-submit-change-request' attribute to differentiate behavior, but this is fragile. If an owner's button is present without this attribute, it will submit without any modal confirmation, which might not be the intended behavior in all cases.

    // Handle Submit Changes button (submit change request - creates PR back to original)
    const submitChangesButton = document.querySelector<HTMLButtonElement>('#submit-changes-button');
    if (submitChangesButton && !submitChangesButton.classList.contains('disabled')) {
      submitChangesButton.addEventListener('click', async () => {
        // Check if this is a submit-change-request action that needs confirmation
        const isSubmitChangeRequest = submitChangesButton.getAttribute('data-submit-change-request') === 'true';

        if (isSubmitChangeRequest) {
          // Get confirmation modal content from data attributes
          const title = submitChangesButton.getAttribute('data-confirm-title') || 'Submit Changes';
          const body = submitChangesButton.getAttribute('data-confirm-body') || 'This will create a pull request with your changes.';
          const confirmText = submitChangesButton.getAttribute('data-confirm-yes') || 'Submit';
          const cancelText = submitChangesButton.getAttribute('data-confirm-cancel') || 'Cancel';

          // Show confirmation modal
          const confirmed = await showForkConfirmModal(title, body, confirmText, cancelText);
          if (!confirmed) {
            return; // User cancelled, do nothing
          }

          // Set submit_change_request to true, fork_and_edit to false
          const forkAndEditField = document.querySelector<HTMLInputElement>('#fork_and_edit');
          const submitChangeRequestField = document.querySelector<HTMLInputElement>('#submit_change_request');
          if (forkAndEditField) forkAndEditField.value = 'false';
          if (submitChangeRequestField) submitChangeRequestField.value = 'true';
        }

        // Update textarea with editor content before submission
        textarea.value = editor.getMarkdown();

        // Submit the form using fetch action to handle JSON redirect response
        await submitFormFetchAction(editForm);
      });
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-31T13:18:45Z

custom/options/locale/locale_en-US.ini

+editor.submit_changes_tooltip = Submit your changes as a Change Request to the original article
+editor.submit_changes_confirm_title = Submit Changes
+editor.submit_changes_confirm_body = This will create a Change Request with your changes. The article owner will be able to review and merge your contribution.
+editor.submit_changes_confirm_yes = Submit Change Request
+editor.submit_changes_confirm_cancel = Cancel
+editor.submit_changes_pr_title = Update %s
+editor.cannot_create_branch = Failed to submit your changes.


The locale string 'editor.submit_changes_tooltip' is defined in the custom locale file but not in the main options/locale/locale_en-US.ini file. This creates inconsistency and could cause missing translations if the custom locale is not always loaded. The locale string should be defined in both places or consolidated into one location.

Copilot · 2025-12-31T13:18:46Z

routers/web/repo/editor.go

+	log.Info("handleSubmitChangeRequest: created CR #%d from %s to %s in %s/%s",
+		changeRequest.Index,
+		branchName,
+		targetRepo.DefaultBranch,
+		targetRepo.OwnerName, targetRepo.Name)


The log message uses 'CR' (Change Request) as an abbreviation without establishing this terminology elsewhere. This is inconsistent with standard Gitea terminology which uses 'PR' for Pull Request. The log message should use consistent terminology to avoid confusion when debugging.

Copilot · 2025-12-31T13:18:46Z

routers/web/repo/editor.go

+
+	// Validate that content is provided
+	if !form.Content.Has() {
+		ctx.JSONError(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, "content is required"))


The error message at line 616 includes a hardcoded string "content is required" as the second argument to the translation function. This hardcoded English text defeats the purpose of using translation functions and makes internationalization inconsistent. The error should either use a separate translation key or remove the hardcoded text.

Suggested change

ctx.JSONError(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, "content is required"))

ctx.JSONError(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, ctx.Tr("repo.editor.content_is_required")))

Copilot · 2025-12-31T13:18:46Z

routers/web/repo/editor.go

+// Returns the created change request, or nil if an error occurred.
+func handleSubmitChangeRequest(ctx *context.Context, form *forms.EditRepoFileForm, parsed *preparedEditorCommitForm[*forms.EditRepoFileForm]) *issues_model.PullRequest {
+	targetRepo := ctx.Repo.Repository
+


The handleSubmitChangeRequest function is missing permission validation to check if the user is authenticated. While the middleware may handle authentication, this function should explicitly verify that ctx.Doer is not nil before proceeding to create branches and PRs in the target repository. The handleForkAndEdit function doesn't have this check either, but this new function should include it for security best practices.

Suggested change

// Ensure the user is authenticated before proceeding

if ctx.Doer == nil {

ctx.JSONError(ctx.Tr("repo.editor.must_be_signed_in"))

return nil

}

Copilot · 2025-12-31T13:18:47Z

routers/web/repo/editor.go

+	// Same-repo CR: both head and base are in the target repository
+	compareInfo, err := pull_service.GetCompareInfo(ctx, targetRepo, targetRepo, gitRepo,


The comment refers to 'CR' (Change Request) which is inconsistent with the terminology used elsewhere in the codebase. Gitea typically uses 'Pull Request' or 'PR'. While 'Change Request' may be domain-specific terminology for this feature, using 'CR' as an abbreviation without first establishing it could be confusing. Consider using 'PR' consistently or explicitly defining 'CR' in a more prominent location.

Copilot · 2025-12-31T13:18:48Z

web_src/js/features/article-editor.ts

+          // Set submit_change_request to true, fork_and_edit to false
+          const forkAndEditField = document.querySelector<HTMLInputElement>('#fork_and_edit');
+          const submitChangeRequestField = document.querySelector<HTMLInputElement>('#submit_change_request');
+          if (forkAndEditField) forkAndEditField.value = 'false';
+          if (submitChangeRequestField) submitChangeRequestField.value = 'true';
        }


The form initializes 'fork_and_edit' to "true" when NeedsFork is true (line 76), and 'submit_change_request' to "false" (line 78). However, when a user clicks the Submit Change Request button, the JavaScript only sets 'submit_change_request' to "true" and 'fork_and_edit' to "false". Since both conditions can theoretically be true in form data if there's a timing issue or if the JavaScript fails to execute properly, the middleware at services/context/permission.go:36 uses an OR condition which could lead to unexpected behavior. The backend handler should validate that only one of these flags is true, not both.

Copilot · 2025-12-31T13:18:48Z

tests/e2e/submit-change-request.test.e2e.ts

+      const submitCRButton = page.locator('#submit-change-request-button');
+      await expect(submitCRButton).toBeHidden();


This test expects a button with ID 'submit-change-request-button' which does not exist in the template. According to the template at custom/templates/shared/repo/article.tmpl line 103, the submit change request button has the ID 'submit-changes-button', not 'submit-change-request-button'. This test will fail because it cannot find the expected button.

Copilot · 2025-12-31T13:18:48Z

tests/e2e/submit-change-request.test.e2e.ts

+      const submitCRButton = page.locator('#submit-change-request-button');
+      await expect(submitCRButton).toBeVisible({timeout: 10000});
+
+      await submitCRButton.click();
+
+      // Modal should appear with confirmation message
+      const modal = page.locator('.ui.g-modal-confirm.modal.visible');
+      await expect(modal).toBeVisible({timeout: 5000});
+
+      // Modal should have appropriate header and content
+      const header = modal.locator('.header');
+      await expect(header).toContainText(/Change Request|Submit/i);
+
+      const content = modal.locator('.content');
+      await expect(content).toBeVisible();
+
+      await context.close();
+    });
+
+    test('canceling modal does not submit changes', async ({browser}, workerInfo) => {
+      const context = await load_logged_in_context(browser, workerInfo, 'user4');
+      const page = await context.newPage();
+
+      await page.goto('/article/user2/example-subject?mode=edit');
+      await page.waitForLoadState('domcontentloaded');
+
+      await expect(page.locator('#article-edit-form')).toBeVisible({timeout: 10000});
+      await expect(page.locator('.toastui-editor').first()).toBeAttached({timeout: 20000});
+
+      const urlBefore = page.url();
+
+      const submitCRButton = page.locator('#submit-change-request-button');
+      await expect(submitCRButton).toBeVisible({timeout: 10000});
+      await submitCRButton.click();
+
+      const modal = page.locator('.ui.g-modal-confirm.modal.visible');
+      await expect(modal).toBeVisible({timeout: 5000});
+
+      // Click cancel button
+      const cancelButton = modal.locator('.actions .cancel.button');
+      await cancelButton.click();
+
+      // Modal should close
+      await expect(modal).not.toBeVisible({timeout: 5000});
+
+      // URL should not change (no redirect to PR)
+      expect(page.url()).toBe(urlBefore);
+
+      await context.close();
+    });
+  });
+
+  test.describe('Accessibility Tests', () => {
+    test('Submit Change Request button is keyboard accessible', async ({browser}, workerInfo) => {
+      const context = await load_logged_in_context(browser, workerInfo, 'user4');
+      const page = await context.newPage();
+
+      await page.goto('/article/user2/example-subject?mode=edit');
+      await page.waitForLoadState('domcontentloaded');
+
+      await expect(page.locator('#article-edit-form')).toBeVisible({timeout: 10000});
+      await expect(page.locator('.toastui-editor').first()).toBeAttached({timeout: 20000});
+
+      const submitCRButton = page.locator('#submit-change-request-button');
+      await expect(submitCRButton).toBeVisible({timeout: 10000});
+
+      // Focus the button and press Enter
+      await submitCRButton.focus();
+      await page.keyboard.press('Enter');
+
+      // Modal should open
+      const modal = page.locator('.ui.g-modal-confirm.modal.visible');
+      await expect(modal).toBeVisible({timeout: 5000});
+
+      await context.close();
+    });
+
+    test('Escape key closes modal', async ({browser}, workerInfo) => {
+      const context = await load_logged_in_context(browser, workerInfo, 'user4');
+      const page = await context.newPage();
+
+      await page.goto('/article/user2/example-subject?mode=edit');
+      await page.waitForLoadState('domcontentloaded');
+
+      await expect(page.locator('#article-edit-form')).toBeVisible({timeout: 10000});
+      await expect(page.locator('.toastui-editor').first()).toBeAttached({timeout: 20000});
+
+      const submitCRButton = page.locator('#submit-change-request-button');
+      await expect(submitCRButton).toBeVisible({timeout: 10000});
+      await submitCRButton.click();
+
+      const modal = page.locator('.ui.g-modal-confirm.modal.visible');
+      await expect(modal).toBeVisible({timeout: 5000});
+
+      // Press Escape to close
+      await page.keyboard.press('Escape');
+
+      await expect(modal).not.toBeVisible({timeout: 5000});
+
+      await context.close();
+    });
+  });
+
+  test.describe('Tooltip Tests', () => {
+    test('Submit Change Request button has informative tooltip', async ({browser}, workerInfo) => {
+      const context = await load_logged_in_context(browser, workerInfo, 'user4');
+      const page = await context.newPage();
+
+      await page.goto('/article/user2/example-subject?mode=edit');
+      await page.waitForLoadState('domcontentloaded');
+
+      const submitCRButton = page.locator('#submit-change-request-button');
+      await expect(submitCRButton).toBeVisible({timeout: 10000});
+
+      // Button should have a tooltip explaining the action
+      await expect(submitCRButton).toHaveAttribute('data-tooltip-content', /change request|pull request|review/i);
+
+      await context.close();
+    });
+  });
+});
+
+test.describe('Submit Change Request - Unauthenticated User', () => {
+  test('unauthenticated user sees sign-in prompt instead of Submit Change Request', async ({page}) => {
+    await page.goto('/article/user2/example-subject?mode=edit');
+    await page.waitForLoadState('domcontentloaded');
+
+    // Unauthenticated user should see a disabled button with sign-in message
+    const signInButton = page.locator('button.ui.primary.button.disabled');
+    await expect(signInButton).toBeVisible({timeout: 10000});
+
+    // Should contain sign-in text
+    await expect(signInButton).toContainText(/Sign in/i);
+
+    // Submit Change Request button should not be visible
+    const submitCRButton = page.locator('#submit-change-request-button');
+    await expect(submitCRButton).toBeHidden();


All references to 'submit-change-request-button' throughout this test file are incorrect. The actual button ID in the template is 'submit-changes-button'. This will cause all these tests to fail as they cannot locate the expected button element.

Copilot · 2025-12-31T13:18:48Z

tests/integration/submit_change_request_test.go

+		// Try to submit with submit_change_request=true in form but not in query
+		// This should fail because the middleware checks the query param
+		form := map[string]string{
+			"_csrf":                 htmlDoc.GetCSRF(),
+			"last_commit":           htmlDoc.GetInputValueByName("last_commit"),
+			"tree_path":             "README.md",
+			"content":               "Test content",
+			"commit_choice":         "direct",
+			"submit_change_request": "true",
+		}
+
+		req = NewRequestWithValues(t, "POST", editURL, form)
+		resp = sessionNonOwner.MakeRequest(t, req, NoExpectedStatus)
+
+		// Should get 404 because the middleware didn't bypass permission check
+		// (the query param wasn't set on the GET request)
+		assert.Equal(t, http.StatusNotFound, resp.Code,
+			"Should fail without submit_change_request query param on GET")


The test comment on line 90 states "This should fail because the middleware checks the query param", but the middleware implementation at services/context/permission.go:36 uses ctx.Req.FormValue() which checks both query parameters AND form data. This means the test's expectation might be wrong, or the middleware behavior needs to be updated to only check query parameters. The current implementation would allow bypassing the permission check by setting submit_change_request=true in the form data alone, which may not be the intended security behavior.

Copilot · 2025-12-31T13:18:49Z

custom/templates/shared/repo/article.tmpl

-                                            <button type="button" class="ui primary button disabled">
-                                                {{svg "octicon-check" 16 "tw-mr-1"}}
+                                            {{/* Submit Change Request - creates fork + branch + commit + PR */}}
+                                            <button type="button" id="submit-changes-button" class="ui primary button"


The Submit Change Request button at line 103 lacks an explicit 'type' attribute. While buttons default to type="button" in most contexts, it's a best practice to explicitly specify type="button" to prevent accidental form submission if the button is within a form element and the JavaScript event handler fails.

pedrogaudencio added 8 commits December 23, 2025 00:32

editor: implement submit-change-request for article contributions

683b99c

* 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

eb94795

* update middleware to bypass permission check for submit_change_request=true * enable non-owners to submit change requests

editor: fix log message format

56fc001

editor: validate repository allows pull requests

5930277

editor: add content validation

f7cde0d

tests: update fork-article-modal tests to use new button ID

195e8aa

tests: add submit-change-request workflow tests

a2771ee

* 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

Merge branch 'master' into change-request-submit-edits

640bd80

pedrogaudencio requested a review from Copilot December 31, 2025 13:06

pedrogaudencio self-assigned this Dec 31, 2025

github-actions bot added modifies/go modifies/frontend modifies/translation labels Dec 31, 2025

Copilot started reviewing on behalf of pedrogaudencio December 31, 2025 13:07 View session

Copilot AI reviewed Dec 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Change request: enable submitting article edits #101

Change request: enable submitting article edits #101

pedrogaudencio commented Dec 31, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	ctx.JSONError(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, "content is required"))
	ctx.JSONError(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, ctx.Tr("repo.editor.content_is_required")))

+	// Ensure the user is authenticated before proceeding
+	if ctx.Doer == nil {
+		ctx.JSONError(ctx.Tr("repo.editor.must_be_signed_in"))
+		return nil
+	}

		// Same-repo CR: both head and base are in the target repository
		compareInfo, err := pull_service.GetCompareInfo(ctx, targetRepo, targetRepo, gitRepo,

		const submitCRButton = page.locator('#submit-change-request-button');
		await expect(submitCRButton).toBeHidden();

Change request: enable submitting article edits #101

Are you sure you want to change the base?

Change request: enable submitting article edits #101

Conversation

pedrogaudencio commented Dec 31, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants