Corrections from review

- destroy_info: remove superfluous JS confirm dialog.
- destroy_info: change notification type into severe warning
- destroy_info: Remove delete button highlight and back button
                for consistency with styleguide.
                TODO: think of other ways to guide the user back
                      (also applies to project destroy_info)

- remove typo in 'repositories.errors.unauthorized'
- empty repository: Render warning view (repositories/empty)
                    instead of 500 exception notification.
- revert reformat on code changed in rails4 branch.
- create repository: Show managed flash part only when the repository is
                     managed
- delete repository: Show error when configuration no longer allows
                     manageable (other ways tbd.)
- repository controller: JS redirect wrapped inside format.js
- repository settings: Select hidden when instance exists

Specs:
- Add routing spec for `get #destroy_info`

app/controllers/repositories_controller.rb

@@ -79,11 +79,14 @@ class RepositoriesController < ApplicationController
     if service.build_and_save
       @repository = service.repository
       flash[:notice] = l('repositories.create_successful')
+      flash[:notice] << (' ' + l('repositories.create_managed_delay')) if @repository.managed?
     else
       flash[:error] = service.build_error
     end
 
-    render js: "window.location = '#{settings_repository_tab_path}'"
+    respond_to do |format|
+      format.js { render js: "window.location = '#{settings_repository_tab_path}'" }
+    end
   end
 
   def committers
@@ -320,6 +323,8 @@ class RepositoriesController < ApplicationController
         raise InvalidRevisionParam
       end
     end
+  rescue OpenProject::Scm::Exceptions::ScmEmpty
+    render 'empty'
   rescue ActiveRecord::RecordNotFound
     render_404
   rescue InvalidRevisionParam

app/models/repository.rb

@@ -170,18 +170,14 @@ class Repository < ActiveRecord::Base
   # Default behaviour is to search in cached changesets
   def latest_changesets(path, _rev, limit = 10)
     if path.blank?
-      changesets.find(:all,
-                      include: :user,
-                      order: "#{Changeset.table_name}.committed_on DESC, "\
-                             "#{Changeset.table_name}.id DESC",
-                      limit: limit)
+      changesets.find(:all, include: :user,
+                            order: "#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC",
+                            limit: limit)
     else
-      changes.find(:all,
-                   include: { changeset: :user },
-                   conditions: ['path = ?', path.with_leading_slash],
-                   order: "#{Changeset.table_name}.committed_on DESC, "\
-                          "#{Changeset.table_name}.id DESC",
-                   limit: limit).map(&:changeset)
+      changes.find(:all, include: { changeset: :user },
+                         conditions: ['path = ?', path.with_leading_slash],
+                         order: "#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC",
+                         limit: limit).map(&:changeset)
     end
   end
 
@@ -354,7 +350,7 @@ class Repository < ActiveRecord::Base
       true
     else
       raise OpenProject::Scm::Exceptions::RepositoryUnlinkError.new(
-        service.localized_rejected_reason
+         I18n.t('repositories.errors.unlink_failed_unmanageable')
       )
     end
   end

app/views/repositories/_settings.html.erb

@@ -36,6 +36,8 @@ See doc/COPYRIGHT.rdoc for more details.
              builder: TabularFormBuilder,
              html: { class: 'form' } do |f| %>
   <%= error_messages_for 'repository' %>
+  <%# Hide the select for existing repositories %>
+  <% if @repository.new_record? %>
   <div class="form--field -required">
     <%= label_tag('scm_vendor', l('repositories.scm_vendor'), class: "form--label") %>
     <div class="form--field-container">
@@ -44,6 +46,9 @@ See doc/COPYRIGHT.rdoc for more details.
       </div>
     </div>
   </div>
+  <% end %>
+
+  <%# Show (selected) type options %>
   <% unless @repository.nil? %>
       <%= render partial: "/repositories/settings/vendor_form",
                  locals: { form: f, repository: @repository, vendor: vendor_name(@repository) } %>

app/views/repositories/destroy_info.html.erb

@@ -27,7 +27,7 @@ See doc/COPYRIGHT.rdoc for more details.
 
 ++#%>
 <%= toolbar title: l(:label_confirmation) %>
-<div class="notification-box <%= @repository.managed? ? '-error' : '-warning' %>">
+<div class="notification-box -warning <%= @repository.managed? ? '-severe' : '' %>">
   <div class="notification-box--content">
     <p><strong><%= l('repositories.destroy.title') %></strong><br /></p>
     <p>
@@ -42,17 +42,11 @@ See doc/COPYRIGHT.rdoc for more details.
     </p>
     <p>
       <%= link_to project_repository_path(@project),
-        :confirm => l(:text_are_you_sure),
         :method => :delete,
-        :class => 'button -highlight' do %>
+        :class => 'button' do %>
         <i class="button--icon icon-delete"></i>
         <span class="button--text"><%= l(:button_delete) %></span>
       <% end %>
-      <%= link_to @back_link,
-        :class => 'button' do %>
-        <i class="button--icon icon-close"></i>
-        <span class="button--text"><%= l(:button_cancel) %></span>
-      <% end %>
     </p>
   </div>
 </div>

app/views/repositories/empty.html.erb

@@ -0,0 +1,36 @@
+<%#-- copyright
+OpenProject is a project management system.
+Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License version 3.
+
+OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+Copyright (C) 2006-2013 Jean-Philippe Lang
+Copyright (C) 2010-2013 the ChiliProject Team
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 2
+of the License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+See doc/COPYRIGHT.rdoc for more details.
+
+++#%>
+
+<div class="notification-box -warning">
+  <div class="notification-box--content">
+    <p><%= l('repositories.errors.exception_title',
+             message: l('repositories.errors.empty_repository')) %></p>
+    <%# TODO: Add checkout instructions when included in core %>
+  </div>
+</div>

config/locales/en.yml

@@ -1249,7 +1249,8 @@ en:
 
   repositories:
     checkout_instructions: "Checkout instructions"
-    create_successful: "The repository has been registered. If the repository is managed, it will be available shortly."
+    create_managed_delay: "Please note: The repository is managed, it is created asynchronously on the disk and will be available shortly."
+    create_successful: "The repository has been registered."
     delete_sucessful: "The repository has been deleted."
     destroy:
       title: "Are you absolutely sure?"
@@ -1258,11 +1259,12 @@ en:
       linked_text: "You're about to remove the linked repository %{url} from the project %{project_name}.\nNote: This will NOT delete the contents of this repository, as it is not managed by OpenProject."
     errors:
       build_failed: "Unable to create the repository with the selected configuration. %{reason}"
-      empty_repository: "The repository exists, but is empty. It does not contain any revisions."
+      empty_repository: "The repository exists, but is empty. It does not contain any revisions yet."
       exists_on_filesystem: "The repository directory exists already on filesystem."
       not_manageable: "This repository vendor cannot be managed by OpenProject."
-      unauthorized: "You're not authorized to acecss the repository or the credentials are invalid."
+      unauthorized: "You're not authorized to access the repository or the credentials are invalid."
       unavailable: "The repository is unavailable."
+      unlink_failed_unmanageable: "The repository could not be deleted, because it is no longer manageable by OpenProject."
       exception_title: "Cannot access the repository: %{message}"
       disabled_or_unknown_vendor: "The SCM vendor %{vendor} is disabled or no longer available."
     git:

spec/features/repositories/repository_settings_spec.rb

@@ -66,12 +66,10 @@ describe 'Repository Settings', type: :feature, js: true do
       find('a.icon-delete', text: I18n.t(:button_delete)).click
 
       # Confirm the notification warning
-      warning = (type == 'managed') ? '-error' : '-warning'
+      warning = (type == 'managed') ? '-warning.-severe' : '-warning'
       expect(page).to have_selector(".notification-box.#{warning}")
       find('a', text: I18n.t(:button_delete)).click
 
-      # Confirm the popup
-      page.driver.browser.switch_to.alert.accept
       vendor = find('select[name="scm_vendor"]')
       expect(vendor).not_to be_nil
       expect(vendor.value).to be_empty

spec/routing/repositories_routing_spec.rb

@@ -257,4 +257,12 @@ describe RepositoriesController, type: :routing do
                                                                      project_id: 'testproject')
     }
   end
+
+  describe 'destroy_info' do
+    it {
+      expect(get('/projects/testproject/repository/destroy_info')).to route_to(controller: 'repositories',
+                                                                     action: 'destroy_info',
+                                                                     project_id: 'testproject')
+    }
+  end
 end