svn-submit: rm repository_external_commits_only

app/controllers/submissions_controller.rb

@@ -260,19 +260,20 @@ def populate_submissions_table
   # when the state stored in the cookie exceeds 4k in serialized
   # form. This was happening prior to the fix of Github issue #30.
   def update_files
+    assignment_id = params[:assignment_id]
+    @assignment = Assignment.find(assignment_id)
+    unless @assignment.allow_web_submits
+      raise t('student.submission.external_submit_only')
+    end
+
     # We'll use this hash to carry over some error state to the
     # file_manager view.
     @file_manager_errors = Hash.new
-    assignment_id = params[:assignment_id]
-    @assignment = Assignment.find(assignment_id)
     required_files = AssignmentFile.where(
                            assignment_id: @assignment).pluck(:filename)
     students_filename = []
     @path = params[:path] || '/'
     @grouping = current_user.accepted_grouping_for(assignment_id)
-    if @grouping.repository_external_commits_only?
-      raise I18n.t('student.submission.external_submit_only')
-    end
     unless @grouping.is_valid?
       # can't use redirect_to here. See comment of this action for more details.
       set_filebrowser_vars(@grouping.group, @assignment)

app/models/grouping.rb

@@ -608,15 +608,6 @@ def create_grouping_repository_folder
     end
   end
 
-  # Returns true, if and only if the configured repository setup
-  # allows for externally accessible repositories, in which case
-  # file submissions via the Web interface are not permitted. For
-  # now, this works for Subversion repositories only.
-  def repository_external_commits_only?
-    assignment = self.assignment
-    !assignment.allow_web_submits
-  end
-
   # Should we write repository permissions for this grouping?
   def write_repo_permissions?
     MarkusConfigurator.markus_config_repository_admin?

app/models/student.rb

@@ -238,9 +238,10 @@ def self.hide_students(student_id_list)
         end
         student = Student.find(student_id)
         memberships.each do |membership|
-          group = membership.grouping.group
+          grouping = membership.grouping
+          group = grouping.group
           group.access_repo do |repo|
-            if membership.grouping.repository_external_commits_only? && membership.grouping.is_valid?
+            if grouping.assignment.vcs_submit && grouping.is_valid?
               begin
                 repo.remove_user(student.user_name) # revoke repo permissions
               rescue Repository::UserNotFound
@@ -270,9 +271,10 @@ def self.unhide_students(student_id_list)
         end
         student = Student.find(student_id)
         memberships.each do |membership|
-          group = membership.grouping.group
+          grouping = membership.grouping
+          group = grouping.group
           group.access_repo do |repo|
-            if membership.grouping.repository_external_commits_only? && membership.grouping.is_valid?
+            if grouping.assignment.vcs_submit && grouping.is_valid?
               begin
                 repo.add_user(student.user_name, Repository::Permission::READ_WRITE) # grant repo permissions
               rescue Repository::UserAlreadyExistent

app/views/assignments/student_interface.html.erb

@@ -220,7 +220,7 @@
           </ul>
 
           <% # Display the URL of this group's repository if applicable
-             if @grouping.repository_external_commits_only? %>
+             if @assignment.vcs_submit %>
             <h3><%= t('student.url_group_repository') %></h3>
             <div class='sub_block'>
               <a href='<%= @grouping.group.repository_external_access_url %>'><%= @grouping.group.repository_external_access_url %></a>

app/views/submissions/_react_file_manager_boot_table.js.jsx.erb

@@ -100,11 +100,9 @@
         return f;
       }.bind(this));
 
-      var selectable = <%= not @grouping.repository_external_commits_only? %>;
-
       return (
         <div>
-          <% unless !@grouping.is_valid? || @grouping.repository_external_commits_only? %>
+          <% if @grouping.is_valid? && @assignment.allow_web_submits %>
             <FileManagerActionBox
               selected_files={this.state.selected_files}
               data_file={files_data}
@@ -114,7 +112,7 @@
 
           <Table data={files_data}
             columns={this.props.columns}
-            selectable={selectable}
+            selectable={<%= @assignment.allow_web_submits %>}
             onSelectedRowsChange={this.updateSelectedFiles}
             search_placeholder={'<%= raw I18n.t("student.submission.search_files") %>'}
             ref='table' />
@@ -181,10 +179,8 @@
 
     openModal: function(e) {
       e.preventDefault();
-      <% if (@grouping.is_valid? && !@grouping.repository_external_commits_only?) %>
-        modal_addnew.open();
-        set_onbeforeunload(true);
-      <% end %>
+      modal_addnew.open();
+      set_onbeforeunload(true);
     },
 
     render: function() {

app/views/submissions/file_manager.html.erb

@@ -41,19 +41,21 @@
     <p class='success'><%= flash[:submit_notice] %></p>
   <% end %>
 
-  <div id='directory_selector' <%=raw('style="display:none;"') unless @grouping.repository_external_commits_only? %>>
-    <p>
-      <%= I18n.t('student.submission.current_path') %>
-      <% path_memory = '/'
-         links = []
-         @path.split('/').each do |folder|
-         path_memory = File.join(path_memory, folder)
-         links.push(link_to folder, action: 'file_manager', path: path_memory)
-      end %>
-      <%= link_to '[root]', action: 'file_manager', path: '/' %>
-      <%= links.join('/')%>
-    </p>
-  </div>
+  <% if @assignment.vcs_submit %>
+    <div id='directory_selector'>
+      <p>
+        <%= I18n.t('student.submission.current_path') %>
+        <% path_memory = '/'
+           links = []
+           @path.split('/').each do |folder|
+           path_memory = File.join(path_memory, folder)
+           links.push(link_to folder, action: 'file_manager', path: path_memory)
+        end %>
+        <%= link_to '[root]', action: 'file_manager', path: '/' %>
+        <%= links.join('/')%>
+      </p>
+    </div>
+  <% end %>
 
   <% if flash[:transaction_warning] %>
     <div class='warning'><%= flash[:transaction_warning] %></div>
@@ -137,13 +139,11 @@
                { multipart: true, id: 'file_form' } do %>
     <%= file_field_tag :new_files, name: 'new_files[]', multiple: true %>
 
-    <% unless !@grouping.is_valid? || @grouping.repository_external_commits_only? %>
-      <section class='dialog-actions'>
-        <%= submit_tag(I18n.t(:submit),
-                       disabled: !@grouping.is_valid?,
-                       onclick: 'set_onbeforeunload(false); submitFile(event)') unless @grouping.repository_external_commits_only? %>
-        <%= button_to_function t(:close), 'modal_addnew.close();' %>
-      </section>
-    <% end %>
+    <section class='dialog-actions'>
+      <%= submit_tag t(:submit),
+                     disabled: !@grouping.is_valid?,
+                     onclick: 'set_onbeforeunload(false); submitFile(event)' %>
+      <%= button_to_function t(:close), 'modal_addnew.close();' %>
+    </section>
   <% end %>
 </aside>

spec/models/grouping_spec.rb

@@ -270,24 +270,4 @@ def expect_updated_criteria_coverage_count_eq(expected_count)
       end
     end
   end
-
-  describe '#repository_external_commits_only?' do
-    context 'linked to an assignment allowing web commits' do
-      let(:assignment) { create(:assignment, allow_web_submits: true) }
-      let(:grouping) { create(:grouping, assignment_id: assignment.id) }
-
-      it 'returns false for external accessible repository' do
-        expect(grouping.repository_external_commits_only?).to be_falsey
-      end
-    end
-
-    context 'linked to an assignment not allowing web commits' do
-      let(:assignment) { create(:assignment, allow_web_submits: false) }
-      let(:grouping) { create(:grouping, assignment_id: assignment.id) }
-
-      it 'returns true for external accessible repository' do
-        expect(grouping.repository_external_commits_only?).to be_truthy
-      end
-    end
-  end
 end

test/unit/student_test.rb

@@ -218,7 +218,7 @@ class StudentTest < ActiveSupport::TestCase
 
     should 'hide students and have the repo remove them' do
       # Mocks to enter into the if
-      Grouping.any_instance.stubs(:repository_external_commits_only?).returns(true)
+      Assignment.any_instance.stubs(:vcs_submit).returns(true)
       Grouping.any_instance.stubs(:is_valid?).returns(true)
 
       # Mock the repository and expect :remove_user with the student's user_name
@@ -233,7 +233,7 @@ class StudentTest < ActiveSupport::TestCase
 
     should 'not error when user is not found on hide and remove' do
       # Mocks to enter into the if that leads to the call to remove the student
-      Grouping.any_instance.stubs(:repository_external_commits_only?).returns(true)
+      Assignment.any_instance.stubs(:vcs_submit).returns(true)
       Grouping.any_instance.stubs(:is_valid?).returns(true)
 
       # Mock the repository and raise Repository::UserNotFound
@@ -282,7 +282,7 @@ class StudentTest < ActiveSupport::TestCase
 
     should 'unhide without error when users already exists in repo' do
       # Mocks to enter into the if
-      Grouping.any_instance.stubs(:repository_external_commits_only?).returns(true)
+      Assignment.any_instance.stubs(:vcs_submit).returns(true)
       Grouping.any_instance.stubs(:is_valid?).returns(true)
 
       # Mock the repository and raise Repository::UserNotFound