Skip to content

Change request revision rounds#154

Merged
taoeffect merged 167 commits intomasterfrom
change-request-revision-rounds
Mar 10, 2026
Merged

Change request revision rounds#154
taoeffect merged 167 commits intomasterfrom
change-request-revision-rounds

Conversation

@pedrogaudencio
Copy link
Collaborator

@pedrogaudencio pedrogaudencio commented Feb 27, 2026

  • Files tab edit button logic
  • emit push comment after commit
  • sync branches to db before editing Change Request files
  • add submit edit in change request
  • add edit tab on request changes
  • fix edit tab order and copy
  • fix stale refs/pull/N/head after SubmitPullEditPost
  • update missing logic
  • test all features

Closes /issues/145

Notes: depends on /pull/152

Co-authored by: Claude Opus 4.6, Sonnet 4.6 and Haiku 4.5


Open with Devin

* 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
* 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
* 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
* run subject ownership check and fork detection concurrently
* add error for subject ownership violations
* validate subject ownership before forking
* fix POST handler to validate subject ownership
* create IDX_repository_owner_subject and IDX_repository_owner_fork
* optimizes GetRepositoryByOwnerIDAndSubjectID() and GetForkedRepo() queries
* 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
* simplify fork_and_edit hidden field to only use .NeedsFork
* extract repeated nil-guard pattern into $subjectName template variable
* 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
* 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
* 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
* 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
* 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
* 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
…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
* update GetForkedRepo signature to return (*Repository, error)
* update all callers to handle the new error return value
* fixes silent error swallowing
* fix modal promise resolution pattern
* add PathEscapeSegments to article template URL
* update test comment to list all removed special characters
* trim commit_summary form field before evaluating if it's empty
* add test to verify whitespace-only commit_summary uses default message
Copilot AI review requested due to automatic review settings March 9, 2026 13:15

This comment was marked as resolved.

… 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
… 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
devin-ai-integration[bot]

This comment was marked as resolved.

Copilot AI review requested due to automatic review settings March 9, 2026 16:49

This comment was marked as resolved.

* 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
…CommitID

* add  to the owner's edit form to satisfy server-side validation that rejects empty commit_choice with HTTP 400
devin-ai-integration[bot]

This comment was marked as resolved.

…StaleCommitID

* 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
Copilot AI review requested due to automatic review settings March 9, 2026 20:37
devin-ai-integration[bot]

This comment was marked as resolved.

This comment was marked as resolved.

* replace GetCommit(branchName) with GetBranchCommit() (correct API)
* fix stale-review test: expect 404 from CommitID gate, add fresh review step before testing stale POST
* 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
* 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
Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @pedrogaudencio!

@taoeffect taoeffect merged commit 0dc4efd into master Mar 10, 2026
33 checks passed
@taoeffect taoeffect deleted the change-request-revision-rounds branch March 10, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add revision rounds for change requests

4 participants