EE-specific changes for gitlab-org/gitlab-ee#1137
Summary
- gitlab-org/gitlab-ee#1137 is a
technical debt
issue to clean up the EE protected branch access levels (for users and groups) implementation. - Some of this cleanup bleeds over to code shared by CE and EE. This portion is covered in this CE merge request: gitlab-org/gitlab-ce!7821
References
- Closes #1137 (closed)
Tasks
- [#1137 (closed)/!7821/!927 (merged)] Follow-up from restricting pushes / merges by group
-
Implementation -
Prefer validates
with:uniqueness
option! -
Could this be moved into ProtectedBranchAccess
? -
Please name controller action specs after the method and action: describe "GET project_groups"
-
I think we need more extensive integration specs here -
Does this need to be a proc?
-
-
Tests -
CE Passing -
EE Passing
-
-
Meta -
CHANGELOG entry created -
EE branch has no merge conflicts with EE master
-
CE branch has no merge conflicts with CE master
-
Squashed related commits together
-
-
Review -
CE Endboss -
EE Endboss -
Break javascript, make sure integration specs catch the failure
-
-
-
Merge -
CE -
EE
-
-
Merge request reports
Activity
Mentioned in merge request !926 (closed)
@DouweM: You commented in the original thread (https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/645#note_16391229) about needing more extensive integration specs in the
access_control_ee_spec
file.Did you have any specific tests in mind? In my opinion, the current integration tests are sufficient, since most of the details are tested in
git_access_spec
. What do you think?@timothyandrew I don't remember what I meant exactly, but I think we found a couple of UX/frontend bugs and inconsistencies soon after releasing that feature, and the steps to get them would ideally have been covered by feature specs, since they are supposed to test the flow of the feature from the user's perspective.
@DouweM: That makes sense. I'll try and break the UI by modifying the Javascript, and verify that we have failures. If we do, we're okay, and if we don't, it should be easy to pinpoint what needs to be added.
@timothyandrew Sounds good. Generally, every flow that the user can be expected to take should be covered