Database migration can lead to invalid group record

Zendesk issue: https://gitlab.zendesk.com/agent/tickets/40000

I encountered this issue while troubleshooting LDAP group sync. We learned that the reason the group wasn't syncing is because the state machine was failing to update. The reason for the failed state update is the group's validations were failing. Specifically, the group visibility was set to private while there were projects within the group that were internal. The validation at https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/models/group.rb#L24 is what is failing.

Then I started looking at the database migrations and notice something potentially troublesome. The problem is at https://gitlab.com/gitlab-org/gitlab-ee/blob/master/db/migrate/20160301124843_add_visibility_level_to_groups.rb#L10. If an instance already had an allowed visibility level set to 'private' this update would cause all groups to be set to private as well. That seems OK on the surface, but this does not guarantee that all projects are also already private. I'm guessing the customer already had projects that weren't private at the time they decided to restrict visibility to private-only, which led to this sort of inconsistency. However, the database migration blindly updates the group's visibility without checking that validations pass.

What can we do to improve this? Should the original migration be updated so future users don't run in to this? Do we need a subsequent migration to correct the problem? Would the correct way to handle this have been to set the group to the same visibility as the highest project visibility?

@DouweM @stanhu Do you have thoughts on the best way to go forward?