Skip to content
Snippets Groups Projects

Fix renaming repository when name contains invalid chars under settings

Merged Fix renaming repository when name contains invalid chars under settings
fix-19538 into master
4 unresolved threads
Merged Douglas Barbosa Alexandre requested to merge fix-19538 into master
4 unresolved threads

What does this MR do?

Keeps invalid projects suitable for use in URLs

Are there points in the code the reviewer needs to double check?

No.

Why was this MR needed?

500 internal error is displayed without these changes.

What are the relevant issue numbers?

Fixes #19538 (closed)

Screenshots (if relevant)

  • Before Project settings 1 2

  • After Project settings 5 6

  • Before Rename repository 3 4

  • After Rename repository 1

Does this MR meet the acceptance criteria?

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
app/services/projects/update_service.rb
23 23 if project.previous_changes.include?('path')
24 24 project.rename_repo
25 25 end
26 else
27 restore_attributes
  • Sean McGivern
    Sean McGivern @smcgivern started a thread on an old version of the diff
    • spec/services/projects/update_service_spec.rb
    139 139 end
    140 140 end
    141 141
    142 context 'for invalid project path/name' do
    143 it 'resets to previous values to keep project in a valid state' do
  • Sean McGivern
    Sean McGivern @smcgivern ·
    Contributor

    @dbalexandre looks good, just a couple of questions.

  • Sean McGivern Reassigned to @dbalexandre

    Reassigned to @dbalexandre

  • Douglas Barbosa Alexandre Added 3 commits:

    Added 3 commits:

    • 2d2fe2a4 - Fix renaming repository when name contains invalid chars under settings
    • 2d437cf8 - Add feature specs for edit project settings
    • 0d74236b - Update CHANGELOG
  • Douglas Barbosa Alexandre Added 34 commits:

    Added 34 commits:

    • 0d74236b...0c799be6 - 30 commits from branch master
    • 5ea92496 - Fix renaming repository when name contains invalid chars under settings
    • 9dfc5b77 - Add feature specs for edit project settings
    • e20a92dc - Update CHANGELOG
    • 1c8cf120 - Fix Project#to_param to keep invalid project suitable for use in URLs
    Toggle commit list
  • Douglas Barbosa Alexandre
    Douglas Barbosa Alexandre @dbalexandre ·
    Author Maintainer

    Do we need to do this? I find your last screenshot a bit confusing, because the project name is valid but the error says it's invalid.

    @smcgivern I've updated the MR with a better approach to handle this. Could you take another look?

  • Douglas Barbosa Alexandre Reassigned to @smcgivern

    Reassigned to @smcgivern

  • Douglas Barbosa Alexandre Added 1 commit:

    Added 1 commit:

    • 9865b8e6 - Fix Project#to_param to keep invalid project suitable for use in URLs
  • Douglas Barbosa Alexandre Marked the task All builds are passing as completed

    Marked the task All builds are passing as completed

  • Douglas Barbosa Alexandre Marked the task Branch has no merge conflicts with master (if you do - rebase it please) as completed

    Marked the task Branch has no merge conflicts with master (if you do - rebase it please) as completed

  • Sean McGivern
    Sean McGivern @smcgivern ·
    Contributor

    @dbalexandre looks good to me! @rspeicher can you take a look please?

  • Sean McGivern Reassigned to @rspeicher

    Reassigned to @rspeicher

  • Robert Speicher
    Robert Speicher @rspeicher started a thread on an old version of the diff
    • spec/models/project_spec.rb
    372 372
    373 373 it { expect(@project.to_param).to eq('gitlabhq') }
    374 374 end
    375
    376 context 'with invalid path' do
    377 it 'returns previous path to keep project suitable for use in URLs when persisted' do
    378 project = create(:empty_project, path: 'gitlab')
    379 project.path = 'foo&bar'
    380
    381 project.valid?
  • Robert Speicher
    Robert Speicher @rspeicher started a thread on an old version of the diff
    • app/views/projects/update.js.haml
    1 - if @project.valid?
    1 - if @project.errors.blank?
  • Robert Speicher Reassigned to @dbalexandre

    Reassigned to @dbalexandre

  • Douglas Barbosa Alexandre Added 49 commits:

    Added 49 commits:

    • 9865b8e6...ac7f3e6a - 45 commits from branch master
    • 487dfad6 - Fix renaming repository when name contains invalid chars under settings
    • 7d483f8c - Add feature specs for edit project settings
    • 6dfaf4fe - Update CHANGELOG
    • ea4d6c87 - Fix Project#to_param to keep invalid project suitable for use in URLs
    Toggle commit list
  • Robert Speicher Enabled an automatic merge when the build for ea4d6c87 succeeds

    Enabled an automatic merge when the build for ea4d6c87 succeeds

  • Robert Speicher mentioned in commit 373145fb

    mentioned in commit 373145fb

  • Robert Speicher Status changed to merged

    Status changed to merged

  • Douglas Barbosa Alexandre Reassigned to @rspeicher

    Reassigned to @rspeicher

  • Douglas Barbosa Alexandre
    Douglas Barbosa Alexandre @dbalexandre ·
    Author Maintainer

    @rspeicher I've addressed your comments. Can you take another look?

  • Robert Speicher Milestone changed to %8.11

    Milestone changed to %8.11

  • Robert Speicher Mentioned in commit ubudzisz/gitlab-ce@373145fb

    Mentioned in commit ubudzisz/gitlab-ce@373145fb

    • Please register or sign in to reply