From 83add381594bfa7260c6f58cc5e9dff70e407ce8 Mon Sep 17 00:00:00 2001 From: Silin Dmitry Date: Tue, 7 May 2024 00:34:23 +0300 Subject: [PATCH 1/4] Updated error text for incorrect branch name when creating Changelog: changed --- app/assets/javascripts/new_branch_form.js | 9 +++++---- .../projects/branches/user_creates_branch_spec.rb | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js index 34383cb8ada706..47852759905814 100644 --- a/app/assets/javascripts/new_branch_form.js +++ b/app/assets/javascripts/new_branch_form.js @@ -1,6 +1,7 @@ /* eslint-disable func-names, no-return-assign, @gitlab/require-i18n-strings */ const NAME_ERROR_CLASS = 'gl-border-red-500'; +const RESTRICTION_BASE_PREFIX = 'Branch name cannot'; export default class NewBranchForm { constructor(form) { @@ -26,22 +27,22 @@ export default class NewBranchForm { setupRestrictions() { const startsWith = { pattern: /^(\/|\.)/g, - prefix: "can't start with", + prefix: `${RESTRICTION_BASE_PREFIX} start with`, conjunction: 'or', }; const endsWith = { pattern: /(\/|\.|\.lock)$/g, - prefix: "can't end in", + prefix: `${RESTRICTION_BASE_PREFIX} end in`, conjunction: 'or', }; const invalid = { pattern: /(\s|~|\^|:|\?|\*|\[|\\|\.\.|@\{|\/{2,}){1}/g, - prefix: "can't contain", + prefix: `${RESTRICTION_BASE_PREFIX} contain`, conjunction: ', ', }; const single = { pattern: /^@+$/g, - prefix: "can't be", + prefix: `${RESTRICTION_BASE_PREFIX} be`, conjunction: 'or', }; return (this.restrictions = [startsWith, invalid, endsWith, single]); diff --git a/spec/features/projects/branches/user_creates_branch_spec.rb b/spec/features/projects/branches/user_creates_branch_spec.rb index 0260f1359e06e6..410638541dbc43 100644 --- a/spec/features/projects/branches/user_creates_branch_spec.rb +++ b/spec/features/projects/branches/user_creates_branch_spec.rb @@ -108,7 +108,7 @@ click_button("Create branch") expect(page).to have_content('Branch name is invalid') - expect(page).to have_content("can't contain spaces") + expect(page).to have_content("Branch name cannot contain spaces") expect(page).to have_selector('.js-branch-name.gl-border-red-500') end end -- GitLab From a8fc73a3d88121340909137dd9f165232a9c7480 Mon Sep 17 00:00:00 2001 From: Silin Dmitry Date: Tue, 7 May 2024 18:58:40 +0300 Subject: [PATCH 2/4] Fixed failed tests --- spec/frontend/new_branch_spec.js | 84 ++++++++++++++++---------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/spec/frontend/new_branch_spec.js b/spec/frontend/new_branch_spec.js index baff5ebfdb86c8..b23fb63ca0a65b 100644 --- a/spec/frontend/new_branch_spec.js +++ b/spec/frontend/new_branch_spec.js @@ -32,142 +32,144 @@ describe('Branch', () => { it("can't start with a dot", () => { fillNameWith('.foo'); - expectToHaveError("can't start with '.'"); + expectToHaveError("Branch name cannot start with '.'"); }); it("can't start with a slash", () => { fillNameWith('/foo'); - expectToHaveError("can't start with '/'"); + expectToHaveError("Branch name cannot start with '/'"); }); it("can't have two consecutive dots", () => { fillNameWith('foo..bar'); - expectToHaveError("can't contain '..'"); + expectToHaveError("Branch name cannot contain '..'"); }); it("can't have spaces anywhere", () => { fillNameWith(' foo'); - expectToHaveError("can't contain spaces"); + expectToHaveError('Branch name cannot contain spaces'); fillNameWith('foo bar'); - expectToHaveError("can't contain spaces"); + expectToHaveError('Branch name cannot contain spaces'); fillNameWith('foo '); - expectToHaveError("can't contain spaces"); + expectToHaveError('Branch name cannot contain spaces'); }); it("can't have ~ anywhere", () => { fillNameWith('~foo'); - expectToHaveError("can't contain '~'"); + expectToHaveError("Branch name cannot contain '~'"); fillNameWith('foo~bar'); - expectToHaveError("can't contain '~'"); + expectToHaveError("Branch name cannot contain '~'"); fillNameWith('foo~'); - expectToHaveError("can't contain '~'"); + expectToHaveError("Branch name cannot contain '~'"); }); it("can't have tilde anwhere", () => { fillNameWith('~foo'); - expectToHaveError("can't contain '~'"); + expectToHaveError("Branch name cannot contain '~'"); fillNameWith('foo~bar'); - expectToHaveError("can't contain '~'"); + expectToHaveError("Branch name cannot contain '~'"); fillNameWith('foo~'); - expectToHaveError("can't contain '~'"); + expectToHaveError("Branch name cannot contain '~'"); }); it("can't have caret anywhere", () => { fillNameWith('^foo'); - expectToHaveError("can't contain '^'"); + expectToHaveError("Branch name cannot contain '^'"); fillNameWith('foo^bar'); - expectToHaveError("can't contain '^'"); + expectToHaveError("Branch name cannot contain '^'"); fillNameWith('foo^'); - expectToHaveError("can't contain '^'"); + expectToHaveError("Branch name cannot contain '^'"); }); it("can't have : anywhere", () => { fillNameWith(':foo'); - expectToHaveError("can't contain ':'"); + expectToHaveError("Branch name cannot contain ':'"); fillNameWith('foo:bar'); - expectToHaveError("can't contain ':'"); + expectToHaveError("Branch name cannot contain ':'"); fillNameWith(':foo'); - expectToHaveError("can't contain ':'"); + expectToHaveError("Branch name cannot contain ':'"); }); it("can't have question mark anywhere", () => { fillNameWith('?foo'); - expectToHaveError("can't contain '?'"); + expectToHaveError("Branch name cannot contain '?'"); fillNameWith('foo?bar'); - expectToHaveError("can't contain '?'"); + expectToHaveError("Branch name cannot contain '?'"); fillNameWith('foo?'); - expectToHaveError("can't contain '?'"); + expectToHaveError("Branch name cannot contain '?'"); }); it("can't have asterisk anywhere", () => { fillNameWith('*foo'); - expectToHaveError("can't contain '*'"); + expectToHaveError("Branch name cannot contain '*'"); fillNameWith('foo*bar'); - expectToHaveError("can't contain '*'"); + expectToHaveError("Branch name cannot contain '*'"); fillNameWith('foo*'); - expectToHaveError("can't contain '*'"); + expectToHaveError("Branch name cannot contain '*'"); }); it("can't have open bracket anywhere", () => { fillNameWith('[foo'); - expectToHaveError("can't contain '['"); + expectToHaveError("Branch name cannot contain '['"); fillNameWith('foo[bar'); - expectToHaveError("can't contain '['"); + expectToHaveError("Branch name cannot contain '['"); fillNameWith('foo['); - expectToHaveError("can't contain '['"); + expectToHaveError("Branch name cannot contain '['"); }); it("can't have a backslash anywhere", () => { fillNameWith('\\foo'); - expectToHaveError("can't contain '\\'"); + expectToHaveError("Branch name cannot contain '\\'"); fillNameWith('foo\\bar'); - expectToHaveError("can't contain '\\'"); + expectToHaveError("Branch name cannot contain '\\'"); fillNameWith('foo\\'); - expectToHaveError("can't contain '\\'"); + expectToHaveError("Branch name cannot contain '\\'"); }); - it("can't contain a sequence @{ anywhere", () => { + it('Branch name cannot contain a sequence @{ anywhere', () => { fillNameWith('@{foo'); - expectToHaveError("can't contain '@{'"); + expectToHaveError("Branch name cannot contain '@{'"); fillNameWith('foo@{bar'); - expectToHaveError("can't contain '@{'"); + expectToHaveError("Branch name cannot contain '@{'"); fillNameWith('foo@{'); - expectToHaveError("can't contain '@{'"); + expectToHaveError("Branch name cannot contain '@{'"); }); it("can't have consecutive slashes", () => { fillNameWith('foo//bar'); - expectToHaveError("can't contain consecutive slashes"); + expectToHaveError('Branch name cannot contain consecutive slashes'); }); it("can't end with a slash", () => { fillNameWith('foo/'); - expectToHaveError("can't end in '/'"); + expectToHaveError("Branch name cannot end in '/'"); }); it("can't end with a dot", () => { fillNameWith('foo.'); - expectToHaveError("can't end in '.'"); + expectToHaveError("Branch name cannot end in '.'"); }); it("can't end with .lock", () => { fillNameWith('foo.lock'); - expectToHaveError("can't end in '.lock'"); + expectToHaveError("Branch name cannot end in '.lock'"); }); it("can't be the single character @", () => { fillNameWith('@'); - expectToHaveError("can't be '@'"); + expectToHaveError("Branch name cannot be '@'"); }); it('concatenates all error messages', () => { fillNameWith('/foo bar?~.'); - expectToHaveError("can't start with '/', can't contain spaces, '?', '~', can't end in '.'"); + expectToHaveError( + "Branch name cannot start with '/', Branch name cannot contain spaces, '?', '~', Branch name cannot end in '.'", + ); }); it("doesn't duplicate error messages", () => { fillNameWith('?foo?bar?zoo?'); - expectToHaveError("can't contain '?'"); + expectToHaveError("Branch name cannot contain '?'"); }); it('removes the error message when is a valid name', () => { -- GitLab From e82543aef81e43ce140fd2c879c8fd3de371454f Mon Sep 17 00:00:00 2001 From: Silin Dmitry Date: Tue, 7 May 2024 22:16:15 +0300 Subject: [PATCH 3/4] Updated message text --- app/assets/javascripts/new_branch_form.js | 4 ++-- spec/frontend/new_branch_spec.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js index 47852759905814..f4506013477ac9 100644 --- a/app/assets/javascripts/new_branch_form.js +++ b/app/assets/javascripts/new_branch_form.js @@ -38,7 +38,7 @@ export default class NewBranchForm { const invalid = { pattern: /(\s|~|\^|:|\?|\*|\[|\\|\.\.|@\{|\/{2,}){1}/g, prefix: `${RESTRICTION_BASE_PREFIX} contain`, - conjunction: ', ', + conjunction: ' or ', }; const single = { pattern: /^@+$/g, @@ -80,7 +80,7 @@ export default class NewBranchForm { }; const errors = this.restrictions.reduce(validator, []); if (errors.length > 0) { - this.branchNameError.textContent = errors.join(', '); + this.branchNameError.textContent = errors.join('. '); this.name.classList.add(NAME_ERROR_CLASS); this.name.focus(); } else { diff --git a/spec/frontend/new_branch_spec.js b/spec/frontend/new_branch_spec.js index b23fb63ca0a65b..eb37cf15b8c5f3 100644 --- a/spec/frontend/new_branch_spec.js +++ b/spec/frontend/new_branch_spec.js @@ -163,7 +163,7 @@ describe('Branch', () => { it('concatenates all error messages', () => { fillNameWith('/foo bar?~.'); expectToHaveError( - "Branch name cannot start with '/', Branch name cannot contain spaces, '?', '~', Branch name cannot end in '.'", + "Branch name cannot start with '/'. Branch name cannot contain spaces or '?' or '~'. Branch name cannot end in '.'", ); }); -- GitLab From 8aa287601b11614a9d81a76d6569f253972248e9 Mon Sep 17 00:00:00 2001 From: Silin Dmitry Date: Tue, 14 May 2024 21:37:47 +0300 Subject: [PATCH 4/4] Removed text variable --- app/assets/javascripts/new_branch_form.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js index f4506013477ac9..ed540303b0ad6f 100644 --- a/app/assets/javascripts/new_branch_form.js +++ b/app/assets/javascripts/new_branch_form.js @@ -1,7 +1,6 @@ /* eslint-disable func-names, no-return-assign, @gitlab/require-i18n-strings */ const NAME_ERROR_CLASS = 'gl-border-red-500'; -const RESTRICTION_BASE_PREFIX = 'Branch name cannot'; export default class NewBranchForm { constructor(form) { @@ -27,22 +26,22 @@ export default class NewBranchForm { setupRestrictions() { const startsWith = { pattern: /^(\/|\.)/g, - prefix: `${RESTRICTION_BASE_PREFIX} start with`, + prefix: 'Branch name cannot start with', conjunction: 'or', }; const endsWith = { pattern: /(\/|\.|\.lock)$/g, - prefix: `${RESTRICTION_BASE_PREFIX} end in`, + prefix: 'Branch name cannot end in', conjunction: 'or', }; const invalid = { pattern: /(\s|~|\^|:|\?|\*|\[|\\|\.\.|@\{|\/{2,}){1}/g, - prefix: `${RESTRICTION_BASE_PREFIX} contain`, + prefix: 'Branch name cannot contain', conjunction: ' or ', }; const single = { pattern: /^@+$/g, - prefix: `${RESTRICTION_BASE_PREFIX} be`, + prefix: 'Branch name cannot be', conjunction: 'or', }; return (this.restrictions = [startsWith, invalid, endsWith, single]); -- GitLab