Fix trailing space issue with merge requests and issues. Fixes #2514

Fixes #2514 (closed) Strip whitespace from merge request and issue titles. Whitespace doesn't inherently cause problems for the title itself. However, in the event display we bold the title to show when it changes from x to y. This caused markdown to not display the event properly. See #2514 (closed) for an example of previous behavior.

Current behavior:

After proposed changes: (Since we strip whitespace I had to make an arbitrary change to the title to even generate an event. This is expected, though)

Added 1 commit:

• 687fe942 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Added 1 commit:

• 70b4bb90 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Added 879 commits:

• 70b4bb90...680b6d88 - 878 commits from branch gitlab-org:master
• 71b137b7 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

@dzaporozhets Changed as requested

@dblessing do same refactoring for issue controller

Added 1 commit:

• 68e88222 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

@dblessing rebase, retrigger builds?

@axil thanks for the ping. I rebased, added CHANGELOG entry, and hope tests pass. I will check back. If they pass will someone please merge? :) This is a nice easy fix that would be good to get merged and done.

Added 653 commits:

• 68e88222...193bc5fb - 652 commits from branch gitlab-org:master
• 0f0f680a - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Reassigned to @dzaporozhets

I assigned @dzaporozhets since he reviewed the code.

@dzaporozhets In this way I'm getting undefined method strip!' for nil:NilClass. Is this how you meant it should work when you commented before? Thanks for helping me learn on this one.

@dblessing its because title might not be present in params. Add extra check strip if title present

@dzaporozhets Now I understand. I misunderstood your original instructions. I had 2 checks - if params[:merge_request] && params[:merge_request][:title] and you said moving it would eliminate the need for one of those. Thanks for clarifying.

Reassigned to @dblessing

@dblessing assign back to me when ready

@dblessing Did this fall through the cracks? I want to get this fixed!

Milestone changed to 8.2

Added 111 commits:

• 0f0f680a...ca25289b - 110 commits from branch gitlab-org:master
• c070f993 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Reassigned to @dzaporozhets

Reassigned to @dblessing

Title changed from Fix trailing space issue with merge requests and issues. Fixes #2514 (closed) to WIP: Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Added 1 commit:

• 519581cb - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Adding the strip! after the permit call results in ForbiddenAttributes error. I went back to the original which works. This is an example of how it was when I tried to put the strip after the permit. If I'm still doing it wrong, let me know.

params.require(:merge_request).permit(
  :title, :assignee_id, :source_project_id, :source_branch,
  :target_project_id, :target_branch, :milestone_id,
  :state_event, :description, :task_num, label_ids: []
)
params[:merge_request][:title].strip! if params[:merge_request][:title].strip!
params

Added 1 commit:

• de53c6d7 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Title changed from WIP: Fix trailing space issue with merge requests and issues. Fixes #2514 (closed) to Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Added 1 commit:

• 752d5280 - Fix trailing space issue with merge requests and issues. Fixes #2514 (closed)

Now I figured it out. Makes total sense now. TIL

Ready for review.

Reassigned to @dzaporozhets

@dblessing tests are red

Reassigned to @dblessing

Reassigned to @dzaporozhets

Tests now passing

Auto-retry would have been amazing here 😉

@dblessing you can upvote https://gitlab.com/gitlab-org/gitlab-ce/issues/3442 :)

Status changed to merged