Allow a GitHub Organization to be used by multiple Classrooms

app/controllers/application_controller/feature_flags_dependency.rb

@@ -9,6 +9,11 @@ def ensure_student_identifier_flipper_is_enabled
     not_found unless student_identifier_enabled?
   end
 
+  def multiple_classrooms_per_org_enabled?
+    logged_in? && current_user.feature_enabled?(:multiple_classrooms_per_org)
+  end
+  helper_method :multiple_classrooms_per_org_enabled?
+
   def public_assistant_landing_page_enabled?
     GitHubClassroom.flipper[:public_assistant_landing_page].enabled?
   end

app/controllers/organizations_controller.rb

@@ -23,6 +23,8 @@ def new
 
   # rubocop:disable MethodLength
   def create
+    return unless validate_multiple_classrooms_on_org
+
     result = Organization::Creator.perform(
       github_id: new_organization_params[:github_id],
       users: new_organization_params[:users]
@@ -120,10 +122,12 @@ def new_organization_params
     params.require(:organization).permit(:github_id).merge(users: [current_user])
   end
 
-  # rubocop:disable AbcSize
+  # rubocop:disable Metrics/AbcSize
   def set_users_github_organizations
     @users_github_organizations = current_user.github_user.organization_memberships.map do |membership|
       {
+        # TODO: Remove `classroom` field after we turn off the feature flag
+        # for multiple classrooms in one org
         classroom:   Organization.unscoped.find_by(github_id: membership.organization.id),
         github_id:   membership.organization.id,
         login:       membership.organization.login,
@@ -132,15 +136,17 @@ def set_users_github_organizations
       }
     end
   end
-  # rubocop:enable AbcSize
+  # rubocop:enable Metrics/AbcSize
 
   # Check if the current user has any organizations with admin privilege,
   # if so add the user to the corresponding classroom automatically.
   def add_current_user_to_organizations
-    @users_github_organizations.each do |organization|
-      classroom = organization[:classroom]
-      if classroom.present? && !classroom.users.include?(current_user)
-        create_user_organization_access(classroom)
+    @users_github_organizations.each do |github_org|
+      user_classrooms = Organization.where(github_id: github_org[:github_id])
+
+      # Iterate over each classroom associate with this github organization
+      user_classrooms.map do |classroom|
+        create_user_organization_access(classroom) unless classroom.users.include?(current_user)
       end
     end
   end
@@ -173,5 +179,15 @@ def transfer_assignments
       assignment.update_attributes(creator_id: new_owner.id)
     end
   end
+
+  def validate_multiple_classrooms_on_org
+    classroom_exists_on_org = Organization.unscoped.find_by(github_id: new_organization_params[:github_id])
+    if classroom_exists_on_org && !multiple_classrooms_per_org_enabled?
+      flash[:error] = "Validation failed: GitHub ID has already been taken"
+      redirect_to new_organization_path
+      return false
+    end
+    true
+  end
 end
 # rubocop:enable Metrics/ClassLength

app/models/concerns/sluggable.rb

@@ -4,16 +4,20 @@ module Sluggable
   extend ActiveSupport::Concern
 
   included do
-    before_validation do
-      slugify
-    end
+    before_validation :slugify
   end
 
   def slugify
-    self.slug = title.parameterize
+    self.slug = name_for_slug.parameterize
   end
 
   def to_param
     slug
   end
+
+  private
+
+  def name_for_slug
+    title
+  end
 end

app/models/github_organization.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+# rubocop:disable Metrics/ClassLength
 class GitHubOrganization < GitHubResource
   def accept_membership(user_github_login)
     return if organization_member?(user_github_login)
@@ -116,6 +117,12 @@ def create_organization_webhook(config: {}, options: {})
     end
   end
 
+  def organization_webhooks
+    GitHub::Errors.with_error_handling do
+      @client.org_hooks(@id)
+    end
+  end
+
   def remove_organization_webhook(webhook_id)
     return if webhook_id.blank?
     GitHub::Errors.with_error_handling do
@@ -152,3 +159,4 @@ def webhook_secret
     Rails.application.secrets.webhook_secret
   end
 end
+# rubocop:enable Metrics/ClassLength

app/models/organization.rb

@@ -17,15 +17,14 @@ class Organization < ApplicationRecord
 
   has_and_belongs_to_many :users
 
-  validates :github_id, presence: true, uniqueness: true
+  validates :github_id, presence: true
 
   validates :title, presence: true
   validates :title, length: { maximum: 60 }
+  validates :title, uniqueness: { scope: :github_id }
 
   validates :slug, uniqueness: true
 
-  validates :webhook_id, uniqueness: true, allow_nil: true
-
   before_destroy :silently_remove_organization_webhook
 
   def all_assignments(with_invitations: false)
@@ -48,15 +47,22 @@ def github_organization
     @github_organization ||= GitHubOrganization.new(github_client, github_id)
   end
 
-  def slugify
-    self.slug = "#{github_id} #{title}".parameterize
+  def name_for_slug
+    "#{github_id} #{title}"
   end
 
   def one_owner_remains?
     users.count == 1
   end
 
+  # Check if we are the last Classroom on this GitHub Organization
+  def last_classroom_on_org?
+    Organization.where(github_id: github_id).length <= 1
+  end
+
   def silently_remove_organization_webhook
+    return true unless last_classroom_on_org?
+
     begin
       github_organization.remove_organization_webhook(webhook_id)
     rescue GitHub::Error => err

app/models/organization/creator.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+# rubocop:disable Metrics/ClassLength
 class Organization
   class Creator
     include Rails.application.routes.url_helpers
@@ -96,19 +97,29 @@ def perform
     # is a user who has the correct token scope.
     #
     # Returns an Integer id, or raises a Result::Error
+    # rubocop:disable MethodLength
+    # rubocop:disable AbcSize
     def create_organization_webhook!
       return unless (user = user_with_admin_org_hook_scope)
 
       begin
+        return existing_webhook_id unless existing_webhook_id.nil?
+
         github_organization = GitHubOrganization.new(user.github_client, github_id)
         webhook = github_organization.create_organization_webhook(config: { url: webhook_url })
 
-        return webhook.id if webhook.try(:id).present?
-        raise GitHub::Error
-      rescue GitHub::Error
+        raise GitHub::Error if webhook.try(:id).nil?
+
+        webhook.id
+      rescue GitHub::Error => e
+        github_webhook_id = process_webhook_error(e.message, github_organization)
+        return github_webhook_id unless github_webhook_id.nil?
+
         raise Result::Error, "Could not create WebHook, please try again."
       end
     end
+    # rubocop:enable AbcSize
+    # rubocop:enable MethodLength
 
     # Internal: Make sure every user being added to the
     # Organization is an admin on GitHub.
@@ -167,7 +178,10 @@ def title
 
       github_client = users.sample.github_client
       github_org = GitHubOrganization.new(github_client, github_id)
-      github_org.name.present? ? github_org.name : github_org.login
+
+      org_identifier = github_org.name.presence || github_org.login
+
+      find_unique_title(org_identifier)
     end
 
     # Internal: Find User that has the `admin:org_hook` scope
@@ -222,5 +236,63 @@ def webhook_url
 
       raise Result::Error, error_message
     end
+
+    # Internal: Check error from GitHub when we try to create the webhook
+    # If the hook already exists get the webhook id from GitHub and return
+    #
+    # Returns a String for the webhook_id or nil
+    def process_webhook_error(message, github_organization)
+      return nil unless message == "Hook: Hook already exists on this organization"
+
+      get_organization_webhook_id(github_organization)
+    end
+
+    # Internal: Get id of webhook already on GitHub
+    #
+    # Returns a String for the webhook_id or nil
+    def get_organization_webhook_id(github_organization)
+      webhooks = github_organization.organization_webhooks
+
+      # There should only be one webhook that Classroom creates in production
+      webhooks.first.id
+    rescue GitHub::Error
+      nil
+    end
+
+    # Internal: Find webhook_id of any existing classrooms with
+    # same github id
+    #
+    # Returns a String for the webhook_id or nil
+    def existing_webhook_id
+      classroom_with_same_org = Organization.find_by(github_id: github_id)
+      classroom_with_same_org ? classroom_with_same_org.webhook_id : nil
+    end
+
+    # Internal: Generates unique name for classroom if default
+    # name has already been taken
+    #
+    # Returns a String with an unique title
+    def find_unique_title(identifier)
+      # First Classroom on Org will have postfix 1
+      base_title = "#{identifier}-classroom-1"
+
+      count = 1
+      until unique_title?(base_title)
+        # Increments count at end of title
+        base_title = base_title.gsub(/\-\d+$/, "") + "-#{count}"
+        count += 1
+      end
+
+      base_title
+    end
+
+    # Internal: Checks if a classroom with the same title and github_id
+    # exists already
+    #
+    # Returns a Boolean on whether duplicate classroom title is
+    def unique_title?(base_title)
+      Organization.where(title: base_title, github_id: github_id).blank?
+    end
   end
 end
+# rubocop:enable Metrics/ClassLength

app/views/organizations/_reset_and_remove_organization_modal.html.erb

@@ -8,11 +8,15 @@
 
   <div class="remodal-description">
     <p>
-      <%= t('views.organizations.delete_consequences_html', organization: sanitize(current_organization.github_organization.login)) %>
+      <% if multiple_classrooms_per_org_enabled? %>
+        <%= t('views.organizations.delete_classroom_consequences_html', organization: sanitize(current_organization.github_organization.login)) %>
+      <% else %>
+        <%= t('views.organizations.delete_org_consequences_html', organization: sanitize(current_organization.github_organization.login)) %>
+      <% end %>
     </p>
   </div>
 
-  <%= form_for current_organization, html: { "data-name" => current_organization.github_organization.login, method: 'delete' } do |f| %>
+  <%= form_for current_organization, html: { "data-name" => current_organization.title, method: 'delete' } do |f| %>
     <dl class="form js-normalize-submit">
       <dt><%= t('views.organizations.delete_confirm') %></dt>
       <dd><input type="text" class="js-input-block form-control input-contrast input-block" autofocus></dd>

app/views/organizations/new.html.erb

@@ -16,7 +16,7 @@
 
         <div class="org-select-grid">
           <% @users_github_organizations.each do |org| %>
-            <% if org[:role] == 'admin' && !org[:classroom].present? %>
+            <% if org[:role] == 'admin' && (!org[:classroom].present? || multiple_classrooms_per_org_enabled?) %>
               <%= render partial: 'organization_select', locals: { org: org } %>
             <% else %>
               <%= render partial: 'disabled_organization_select', locals: { org: org } %>

config/locales/views/organizations/en.yml

@@ -30,8 +30,9 @@ en:
       group_assignments: 'Group Assignments'
       are_you_sure: 'Are you ABSOLUTELY sure?'
       delete_warning: "Unexpected things will happen if you don't read this!"
-      delete_consequences_html: "This action <strong>CANNOT</strong> be undone. Please note that reseting and removing this classroom will delete all repositories and teams under the <strong>%{organization}</strong> account that were created by GitHub Classroom."
-      delete_confirm: 'Please type the name of the organization to confirm'
+      delete_org_consequences_html: "This action <strong>CANNOT</strong> be undone. Please note that reseting and removing this classroom will delete all repositories and teams under the <strong>%{organization}</strong> account that were created by GitHub Classroom."
+      delete_classroom_consequences_html: "This action <strong>CANNOT</strong> be undone. Please note that reseting and removing this classroom will delete all repositories and teams created by this classroom in the <strong>%{organization}</strong> account."
+      delete_confirm: 'Please type the name of the classroom to confirm'
       delete_submit: 'Reset and remove this classroom'
       classroom_settings: 'Classroom settings'
       classroom_profile: 'Classroom profile'

spec/controllers/organizations_controller_spec.rb

@@ -105,6 +105,36 @@
       organization.destroy!
     end
 
+    context "multiple_classrooms_per_org flag not enabled" do
+      before do
+        GitHubClassroom.flipper[:multiple_classrooms_per_org].disable
+      end
+
+      it "will not add an organization that already exists" do
+        existing_organization_options = { github_id: organization.github_id }
+        expect do
+          post :create, params: { organization: existing_organization_options }
+        end.to_not change(Organization, :count)
+      end
+    end
+
+    context "multiple_classrooms_per_org flag is enabled" do
+      before do
+        GitHubClassroom.flipper[:multiple_classrooms_per_org].enable
+      end
+
+      after do
+        GitHubClassroom.flipper[:multiple_classrooms_per_org].disable
+      end
+
+      it "will add a classroom on same organization" do
+        existing_organization_options = { github_id: organization.github_id }
+        expect do
+          post :create, params: { organization: existing_organization_options }
+        end.to change(Organization, :count)
+      end
+    end
+
     it "will fail to add an organization the user is not an admin of" do
       new_organization = build(:organization, github_id: 90)
       new_organization_options = { github_id: new_organization.github_id }
@@ -114,13 +144,6 @@
       end.to_not change(Organization, :count)
     end
 
-    it "will not add an organization that already exists" do
-      existing_organization_options = { github_id: organization.github_id }
-      expect do
-        post :create, params: { organization: existing_organization_options }
-      end.to_not change(Organization, :count)
-    end
-
     it "will add an organization that the user is admin of on GitHub" do
       organization_params = { github_id: organization.github_id, users: organization.users }
       organization.destroy!