Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
f9c732c4
Commit
f9c732c4
authored
Mar 13, 2020
by
Samantha Ming
Committed by
Kushal Pandya
Mar 13, 2020
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Pass target_param to fetch rule
- MR edit - MR create
parent
97dc741e
Changes
4
Hide whitespace changes
Inline
Side-by-side
Showing
4 changed files
with
127 additions
and
3 deletions
+127
-3
ee/app/assets/javascripts/approvals/components/mr_edit/mr_rules.vue
...ets/javascripts/approvals/components/mr_edit/mr_rules.vue
+42
-1
ee/app/assets/javascripts/approvals/stores/modules/mr_edit/actions.js
...s/javascripts/approvals/stores/modules/mr_edit/actions.js
+10
-2
ee/changelogs/unreleased/199790-incorrect-approval-rule-when-update-target-branch.yml
...790-incorrect-approval-rule-when-update-target-branch.yml
+5
-0
ee/spec/frontend/approvals/components/mr_edit/mr_rules_spec.js
...ec/frontend/approvals/components/mr_edit/mr_rules_spec.js
+70
-0
No files found.
ee/app/assets/javascripts/approvals/components/mr_edit/mr_rules.vue
View file @
f9c732c4
...
@@ -7,6 +7,8 @@ import RuleControls from '../rule_controls.vue';
...
@@ -7,6 +7,8 @@ import RuleControls from '../rule_controls.vue';
import
EmptyRule
from
'
./empty_rule.vue
'
;
import
EmptyRule
from
'
./empty_rule.vue
'
;
import
RuleInput
from
'
./rule_input.vue
'
;
import
RuleInput
from
'
./rule_input.vue
'
;
let
targetBranchMutationObserver
;
export
default
{
export
default
{
components
:
{
components
:
{
UserAvatarList
,
UserAvatarList
,
...
@@ -15,6 +17,11 @@ export default {
...
@@ -15,6 +17,11 @@ export default {
EmptyRule
,
EmptyRule
,
RuleInput
,
RuleInput
,
},
},
data
()
{
return
{
targetBranch
:
''
,
};
},
computed
:
{
computed
:
{
...
mapState
([
'
settings
'
]),
...
mapState
([
'
settings
'
]),
...
mapState
({
...
mapState
({
...
@@ -33,6 +40,9 @@ export default {
...
@@ -33,6 +40,9 @@ export default {
canEdit
()
{
canEdit
()
{
return
this
.
settings
.
canEdit
;
return
this
.
settings
.
canEdit
;
},
},
isEditPath
()
{
return
this
.
settings
.
mrSettingsPath
;
},
},
},
watch
:
{
watch
:
{
rules
:
{
rules
:
{
...
@@ -50,8 +60,39 @@ export default {
...
@@ -50,8 +60,39 @@ export default {
immediate
:
true
,
immediate
:
true
,
},
},
},
},
mounted
()
{
if
(
this
.
isEditPath
)
{
this
.
mergeRequestTargetBranchElement
=
document
.
querySelector
(
'
#merge_request_target_branch
'
);
this
.
targetBranch
=
this
.
mergeRequestTargetBranchElement
?.
value
;
if
(
this
.
targetBranch
)
{
targetBranchMutationObserver
=
new
MutationObserver
(
this
.
onTargetBranchMutation
);
targetBranchMutationObserver
.
observe
(
this
.
mergeRequestTargetBranchElement
,
{
attributes
:
true
,
childList
:
false
,
subtree
:
false
,
attributeFilter
:
[
'
value
'
],
});
}
}
},
beforeDestroy
()
{
if
(
this
.
isEditPath
&&
targetBranchMutationObserver
)
{
targetBranchMutationObserver
.
disconnect
();
targetBranchMutationObserver
=
null
;
}
},
methods
:
{
methods
:
{
...
mapActions
([
'
setEmptyRule
'
,
'
addEmptyRule
'
]),
...
mapActions
([
'
setEmptyRule
'
,
'
addEmptyRule
'
,
'
fetchRules
'
]),
onTargetBranchMutation
()
{
const
selectedTargetBranchValue
=
this
.
mergeRequestTargetBranchElement
.
value
;
if
(
this
.
targetBranch
!==
selectedTargetBranchValue
)
{
this
.
targetBranch
=
selectedTargetBranchValue
;
this
.
fetchRules
(
this
.
targetBranch
);
}
},
},
},
};
};
</
script
>
</
script
>
...
...
ee/app/assets/javascripts/approvals/stores/modules/mr_edit/actions.js
View file @
f9c732c4
...
@@ -61,14 +61,22 @@ export const receiveRulesError = () => {
...
@@ -61,14 +61,22 @@ export const receiveRulesError = () => {
createFlash
(
__
(
'
An error occurred fetching the approval rules.
'
));
createFlash
(
__
(
'
An error occurred fetching the approval rules.
'
));
};
};
export
const
fetchRules
=
({
rootState
,
dispatch
})
=>
{
export
const
fetchRules
=
({
rootState
,
dispatch
}
,
targetBranch
=
''
)
=>
{
dispatch
(
'
requestRules
'
);
dispatch
(
'
requestRules
'
);
const
{
mrSettingsPath
,
projectSettingsPath
}
=
rootState
.
settings
;
const
{
mrSettingsPath
,
projectSettingsPath
}
=
rootState
.
settings
;
const
path
=
mrSettingsPath
||
projectSettingsPath
;
const
path
=
mrSettingsPath
||
projectSettingsPath
;
const
params
=
targetBranch
?
{
params
:
{
target_branch
:
targetBranch
,
},
}
:
null
;
return
axios
return
axios
.
get
(
path
)
.
get
(
path
,
params
)
.
then
(
response
=>
mapMRApprovalSettingsResponse
(
response
.
data
))
.
then
(
response
=>
mapMRApprovalSettingsResponse
(
response
.
data
))
.
then
(
settings
=>
({
.
then
(
settings
=>
({
...
settings
,
...
settings
,
...
...
ee/changelogs/unreleased/199790-incorrect-approval-rule-when-update-target-branch.yml
0 → 100644
View file @
f9c732c4
---
title
:
Display correct approval rules based on target branch in Edit MR form
merge_request
:
26053
author
:
type
:
added
ee/spec/frontend/approvals/components/mr_edit/mr_rules_spec.js
View file @
f9c732c4
...
@@ -38,6 +38,16 @@ describe('EE Approvals MRRules', () => {
...
@@ -38,6 +38,16 @@ describe('EE Approvals MRRules', () => {
.
find
(
UserAvatarList
)
.
find
(
UserAvatarList
)
.
props
(
'
items
'
);
.
props
(
'
items
'
);
const
findRuleControls
=
()
=>
wrapper
.
find
(
'
td.js-controls
'
).
find
(
RuleControls
);
const
findRuleControls
=
()
=>
wrapper
.
find
(
'
td.js-controls
'
).
find
(
RuleControls
);
const
setTargetBranchInputValue
=
()
=>
{
const
value
=
'
new value
'
;
const
element
=
document
.
querySelector
(
'
#merge_request_target_branch
'
);
element
.
value
=
value
;
return
value
;
};
const
callTargetBranchHandler
=
MutationObserverSpy
=>
{
const
onTargetBranchMutationHandler
=
MutationObserverSpy
.
mock
.
calls
[
0
][
0
];
return
onTargetBranchMutationHandler
();
};
beforeEach
(()
=>
{
beforeEach
(()
=>
{
store
=
createStoreOptions
(
MREditModule
());
store
=
createStoreOptions
(
MREditModule
());
...
@@ -55,6 +65,66 @@ describe('EE Approvals MRRules', () => {
...
@@ -55,6 +65,66 @@ describe('EE Approvals MRRules', () => {
approvalRules
=
null
;
approvalRules
=
null
;
});
});
describe
(
'
when editing a MR
'
,
()
=>
{
const
initialTargetBranch
=
'
master
'
;
let
targetBranchInputElement
;
let
MutationObserverSpy
;
beforeEach
(()
=>
{
MutationObserverSpy
=
jest
.
spyOn
(
global
,
'
MutationObserver
'
);
targetBranchInputElement
=
document
.
createElement
(
'
input
'
);
targetBranchInputElement
.
id
=
'
merge_request_target_branch
'
;
targetBranchInputElement
.
value
=
initialTargetBranch
;
document
.
body
.
appendChild
(
targetBranchInputElement
);
store
.
modules
.
approvals
.
actions
=
{
fetchRules
:
jest
.
fn
(),
addEmptyRule
:
jest
.
fn
(),
setEmptyRule
:
jest
.
fn
(),
};
store
.
state
.
settings
.
mrSettingsPath
=
'
some/path
'
;
store
.
state
.
settings
.
eligibleApproversDocsPath
=
'
some/path
'
;
store
.
state
.
settings
.
allowMultiRule
=
true
;
});
afterEach
(()
=>
{
targetBranchInputElement
.
parentNode
.
removeChild
(
targetBranchInputElement
);
MutationObserverSpy
.
mockClear
();
});
it
(
'
sets the target branch data to be the same value as the target branch dropdown
'
,
()
=>
{
factory
();
expect
(
wrapper
.
vm
.
targetBranch
).
toBe
(
initialTargetBranch
);
});
it
(
'
updates the target branch data when the target branch dropdown is changed
'
,
()
=>
{
factory
();
const
newValue
=
setTargetBranchInputValue
();
callTargetBranchHandler
(
MutationObserverSpy
);
expect
(
wrapper
.
vm
.
targetBranch
).
toBe
(
newValue
);
});
it
(
'
re-fetches rules when target branch has changed
'
,
()
=>
{
factory
();
setTargetBranchInputValue
();
callTargetBranchHandler
(
MutationObserverSpy
);
expect
(
store
.
modules
.
approvals
.
actions
.
fetchRules
).
toHaveBeenCalled
();
});
it
(
'
disconnects MutationObserver when component gets destroyed
'
,
()
=>
{
const
mockDisconnect
=
jest
.
fn
();
MutationObserverSpy
.
mockImplementation
(()
=>
({
disconnect
:
mockDisconnect
,
observe
:
jest
.
fn
(),
}));
factory
();
wrapper
.
destroy
();
expect
(
mockDisconnect
).
toHaveBeenCalled
();
});
});
describe
(
'
when allow multiple rules
'
,
()
=>
{
describe
(
'
when allow multiple rules
'
,
()
=>
{
beforeEach
(()
=>
{
beforeEach
(()
=>
{
store
.
state
.
settings
.
allowMultiRule
=
true
;
store
.
state
.
settings
.
allowMultiRule
=
true
;
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment