Move 'Branch Existing Package' to its own page

dist/t/spec/features/0040_package_spec.rb

@@ -36,10 +36,10 @@
     within("div#personal-navigation") do
       click_link('Home Project')
     end
-    click_link('Branch Existing Package')
+    click_link('Branch Package')
     fill_in 'linked_project', with: 'openSUSE.org:openSUSE:Tools'
     fill_in 'linked_package', with: 'build'
-    click_button('Accept')
+    click_button('Branch')
     expect(page).to have_content('build.spec')
   end
 

src/api/app/controllers/webui/packages/branches_controller.rb

@@ -1,14 +1,24 @@
 module Webui
   module Packages
     class BranchesController < Packages::MainController
-      before_action :set_project, only: [:new]
+      before_action :set_project, only: [:new, :into]
       before_action :set_package, only: [:new]
 
+      after_action :verify_authorized, only: [:into]
+
       def new
         @revision = params[:revision] || @package.rev
       end
 
+      def into
+        authorize Package.new(project: @project), :create_branch?
+
+        @remote_projects = Project.where.not(remoteurl: nil).pluck(:id, :name, :title)
+      end
+
+      # FIXME: We should completely and solely rely on Pundit for authorization instead of some custom authorization code in the BranchPackage model
       def create
+        # FIXME: We should use strong parameters instead of this custom implementation
         params.fetch(:linked_project) { raise ArgumentError, 'Linked Project parameter missing' }
         params.fetch(:linked_package) { raise ArgumentError, 'Linked Package parameter missing' }
 

src/api/app/controllers/webui/project_controller.rb

@@ -127,7 +127,6 @@ def new_release_request
   end
 
   def show
-    @remote_projects = Project.where.not(remoteurl: nil).pluck(:id, :name, :title)
     @bugowners_mail = @project.bugowner_emails
     @release_targets = @project.release_targets
 

src/api/app/views/webui/packages/branches/_breadcrumb_items.html.haml

@@ -1,12 +1,18 @@
 = render partial: 'webui/project/breadcrumb_items'
 
-%li.breadcrumb-item.text-word-break-all
-  - if flipper_responsive?
-    %i.fa.fa-archive
-    = link_to(@package, package_show_path(@project, @package))
-  - else
-    = link_to truncate(@package.to_s, length: 50), package_show_path(@project, @package)
-
-- if current_page?(new_package_branches_path(package_name: @package, project: @project))
+-# The package doesn't exist when the user is selecting which package to branch into
+  their project, so we don't check other paths below to avoid exceptions
+- if current_page?(project_new_packages_branch_path(@project))
   %li.breadcrumb-item.active{ 'aria-current' => 'page' }
     Branch Package
+- else
+  %li.breadcrumb-item.text-word-break-all
+    - if flipper_responsive?
+      %i.fa.fa-archive
+      = link_to(@package, package_show_path(@project, @package))
+    - else
+      = link_to truncate(@package.to_s, length: 50), package_show_path(@project, @package)
+
+  - if current_page?(new_package_branches_path(package_name: @package, project: @project))
+    %li.breadcrumb-item.active{ 'aria-current' => 'page' }
+      Branch Package

src/api/app/views/webui/packages/branches/into.html.haml

@@ -0,0 +1,33 @@
+- @pagetitle = "Branch Package Into #{@project}"
+
+.row
+  .col
+    .card
+      = render partial: 'webui/project/tabs', locals: { project: @project }
+      .card-body
+        .row
+          .col-12
+            %h3= @pagetitle
+          .col-12.col-md-8
+            %p
+              By branching a package from another project, you add the package and its files to your project in a transparent way.
+              Changes of the files in the original project will also change the files in the branched package in your project.
+              You can add patches to the branched package without affecting the original package.
+            - unless @remote_projects.empty?
+              %p
+                Branches also work across interconnected OBS instances. This OBS instance has
+                #{pluralize(@remote_projects.count, 'interconnect')}.
+              %ul.list-inline
+                - @remote_projects.each do |_id, name, title|
+                  %li.list-inline-item
+                    = link_to("#{name}: #{title}", project_show_path(project: name))
+          .col-12.col-md-9.col-lg-6
+            = form_tag(packages_branches_path) do
+              = render partial: 'webui/shared/autocomplete', locals: { html_id: 'linked_project', label: 'Original project name',
+                                                                       data: { source: autocomplete_projects_path } }
+              = render partial: 'webui/shared/autocomplete', locals: { html_id: 'linked_package', label: 'Original package name',
+                                                                       data: { source: autocomplete_packages_path } }
+              = hidden_field_tag 'target_project', @project
+              = render partial: 'webui/shared/package_branch_form', locals: { show_project_field: false, target_project: @project,
+                                                                              package: nil, revision: nil }
+              = submit_tag('Branch', class: 'btn btn-primary px-4 mt-3')

src/api/app/views/webui/project/_project_packages.html.haml

@@ -19,9 +19,9 @@
               %i.fas.fa-plus-circle.text-primary
               Create Package
           %li.list-inline-item
-            = link_to('#', class: 'nav-link', data: { toggle: 'modal', target: '#new-package-branch-modal' }) do
+            = link_to(project_new_packages_branch_path(project), class: 'nav-link') do
               %i.fas.fa-code-branch.text-primary
-              Branch Existing Package
+              Branch Package
 
 - content_for :ready_function do
   :plain

src/api/app/views/webui/project/responsive_ux/_show_actions.html.haml

@@ -27,6 +27,6 @@
         %i.fas.fa-plus-circle.fa-lg.mr-2
         Create Package
     %li.nav-item
-      = link_to('#', class: 'nav-link', data: { toggle: 'modal', target: '#new-package-branch-modal' }) do
+      = link_to(project_new_packages_branch_path(project), class: 'nav-link') do
         %i.fas.fa-code-branch.fa-lg.mr-2
-        Branch Existing Package
+        Branch Package

src/api/app/views/webui/project/show.html.haml

@@ -80,9 +80,6 @@
   - elsif User.session!.can_modify?(@project)
     = render partial: 'edit_project_dialog', locals: { project: @project }
     = render partial: 'delete_project_dialog', locals: { project: @project }
-
-    - if show_package_actions?
-      = render partial: 'new_package_branch_modal', locals: { project: @project, remote_projects: @remote_projects }
   - elsif !@project.is_locked?
     = render partial: 'webui/request/add_role_request_dialog', locals: { project: @project }
     = render partial: 'webui/request/delete_request_dialog', locals: { project: @project }

src/api/config/routes/webui_routes.rb

@@ -115,7 +115,9 @@
     end
 
     resource :packages, only: [] do
-      resources :branches, controller: 'webui/packages/branches', only: [:create], constraints: cons
+      resources :branches, controller: 'webui/packages/branches', only: [:create], constraints: cons do
+        get '/:project', action: :into, on: :new, as: :project, constraints: cons
+      end
     end
 
     resource :patchinfo, except: [:show], controller: 'webui/patchinfo' do

src/api/spec/features/beta/webui/projects_spec.rb

@@ -96,14 +96,14 @@
     before do
       login user
       visit project_show_path(project)
-      click_menu_link('Actions', 'Branch Existing Package')
+      click_menu_link('Actions', 'Branch Package')
     end
 
     scenario 'an existing package' do
       fill_in('linked_project', with: other_user.home_project_name)
       fill_in('linked_package', with: package_of_another_project.name)
       # This needs global write through
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('Successfully branched package')
       expect(page.current_path).to eq('/package/show/home:Jane/branch_test_package')
@@ -114,7 +114,7 @@
       fill_in('linked_package', with: package_of_another_project.name)
       fill_in('Branch package name', with: 'some_different_name')
       # This needs global write through
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('Successfully branched package')
       expect(page.current_path).to eq("/package/show/#{user.home_project_name}/some_different_name")
@@ -126,7 +126,7 @@
       fill_in('linked_project', with: other_user.home_project_name)
       fill_in('linked_package', with: package_of_another_project.name)
       # This needs global write through
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('You have already branched this package')
       expect(page.current_path).to eq('/package/show/home:Jane/branch_test_package')
@@ -136,10 +136,10 @@
       fill_in('linked_project', with: 'non-existing_package')
       fill_in('linked_package', with: package_of_another_project.name)
 
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('Failed to branch: Package does not exist.')
-      expect(page).to have_current_path(project_show_path('home:Jane'))
+      expect(page).to have_current_path(project_new_packages_branch_path('home:Jane'))
     end
   end
 

src/api/spec/features/webui/projects_spec.rb

@@ -96,14 +96,14 @@
     before do
       login user
       visit project_show_path(project)
-      click_link('Branch Existing Package')
+      click_link('Branch Package')
     end
 
     scenario 'an existing package' do
       fill_in('linked_project', with: other_user.home_project_name)
       fill_in('linked_package', with: package_of_another_project.name)
       # This needs global write through
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('Successfully branched package')
       expect(page.current_path).to eq('/package/show/home:Jane/branch_test_package')
@@ -114,7 +114,7 @@
       fill_in('linked_package', with: package_of_another_project.name)
       fill_in('Branch package name', with: 'some_different_name')
       # This needs global write through
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('Successfully branched package')
       expect(page.current_path).to eq("/package/show/#{user.home_project_name}/some_different_name")
@@ -126,7 +126,7 @@
       fill_in('linked_project', with: other_user.home_project_name)
       fill_in('linked_package', with: package_of_another_project.name)
       # This needs global write through
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('You have already branched this package')
       expect(page.current_path).to eq('/package/show/home:Jane/branch_test_package')
@@ -136,10 +136,10 @@
       fill_in('linked_project', with: 'non-existing_package')
       fill_in('linked_package', with: package_of_another_project.name)
 
-      click_button('Accept')
+      click_button('Branch')
 
       expect(page).to have_text('Failed to branch: Package does not exist.')
-      expect(page).to have_current_path(project_show_path('home:Jane'))
+      expect(page).to have_current_path(project_new_packages_branch_path('home:Jane'))
     end
   end