Merge d80ab9d into a172e69

package/yast2-storage-ng.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Tue Aug 14 11:57:28 UTC 2018 - knut.anderssen@suse.com
+
+- Partitioner: Permit going back when the partition dialog is
+  skipped (bsc#1075443)
+- 4.1.5
+
 -------------------------------------------------------------------
 Mon Aug 13 14:44:32 UTC 2018 - ancor@suse.com
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:		yast2-storage-ng
-Version:	4.1.4
+Version:	4.1.5
 Release:	0
 
 BuildRoot:	%{_tmppath}/%{name}-%{version}-build

src/lib/y2partitioner/actions/add_partition.rb

@@ -46,7 +46,17 @@ def delete_filesystem
       end
 
       def type
-        Dialogs::PartitionType.run(part_controller)
+        case available_types.size
+        when 0
+          raise "No partition type possible"
+        when 1
+          part_controller.type = available_types.first
+          :next
+        else
+          # Only run the dialog if more than one partition type is available
+          # (bsc#1075443)
+          Dialogs::PartitionType.run(part_controller)
+        end
       end
 
       def size
@@ -70,6 +80,10 @@ def size
       def init_transaction
         # The controller object must be created within the transaction
         @part_controller = Controllers::Partition.new(device.name)
+
+        # Once the controller is created we know which steps can be skipped
+        # when going back
+        skip_steps
       end
 
       # @see TransactionWizard
@@ -82,8 +96,6 @@ def sequence_hash
         }.merge(new_blk_device_steps)
       end
 
-      skip_stack :delete_filesystem
-
       def disk_name
         part_controller.disk_name
       end
@@ -175,6 +187,20 @@ def impossible_partition_popup
           )
         )
       end
+
+      # Convenience method for returning the device available partition types
+      #
+      # @return [Array<Y2Storage::PartitionType>]
+      def available_types
+        part_controller.available_partition_types
+      end
+
+      # Convenience method for setting the steps that have to be skipped when
+      # going back
+      def skip_steps
+        self.class.skip_stack :delete_filesystem
+        self.class.skip_stack :type if available_types.size < 2
+      end
     end
   end
 end

src/lib/y2partitioner/actions/controllers/partition.rb

@@ -158,6 +158,16 @@ def new_partition_possible?
           unused_optimal_slots.any?(&:available?)
         end
 
+        # Available partition types for new partitions according to
+        # the available unused slots
+        #
+        # @return [Array<Y2Storage::PartitionType>]
+        def available_partition_types
+          Y2Storage::PartitionType.all.select do |partition_type|
+            unused_optimal_slots.any? { |s| s.possible?(partition_type) }
+          end
+        end
+
         # Title to display in the dialogs during the process
         # @return [String]
         def wizard_title

src/lib/y2partitioner/dialogs/partition_type.rb

@@ -36,7 +36,7 @@ class TypeChoice < CWM::RadioButtons
         def initialize(controller)
           textdomain "storage"
           @controller = controller
-          @slots = controller.unused_optimal_slots
+          @available_types = controller.available_partition_types.map(&:to_s)
         end
 
         # @macro seeAbstractWidget
@@ -51,18 +51,14 @@ def help
         end
 
         def items
-          available_types = Y2Storage::PartitionType.all.map do |ty|
-            [ty.to_s, !@slots.find { |s| s.possible?(ty) }.nil?]
-          end.to_h
-
           [
             # radio button text
             ["primary", _("&Primary Partition")],
             # radio button text
             ["extended", _("&Extended Partition")],
             # radio button text
             ["logical", _("&Logical Partition")]
-          ].find_all { |t, _l| available_types[t] }
+          ].select { |t, _l| @available_types.include?(t) }
         end
 
         # @macro seeAbstractWidget
@@ -73,8 +69,7 @@ def validate
         # @macro seeAbstractWidget
         def init
           # Pick the first one available
-          default_pt = Y2Storage::PartitionType.new(items.first.first)
-          self.value = (@controller.type ||= default_pt).to_s
+          self.value = (@controller.type || @available_types.first).to_s
         end
 
         # @macro seeAbstractWidget
@@ -101,19 +96,6 @@ def contents
         HVSquash(type_choice)
       end
 
-      # Overwrite run. If there is only one type of partition, just select it
-      def run
-        case type_choice.items.size
-        when 0
-          raise "No partition type possible"
-        when 1
-          @controller.type = Y2Storage::PartitionType.new(type_choice.items.first.first)
-          :next
-        else
-          super
-        end
-      end
-
     private
 
       def type_choice

test/y2partitioner/actions/add_partition_test.rb

@@ -39,6 +39,7 @@ def current_graph
 
     allow(Yast::Wizard).to receive(:OpenNextBackDialog)
     allow(Yast::Wizard).to receive(:CloseDialog)
+    allow(subject).to receive(:skip_steps)
   end
 
   subject(:action) { described_class.new(disk) }
@@ -180,9 +181,11 @@ def current_graph
             .and_return(controller)
           # Only to finish
           allow(Y2Partitioner::Dialogs::PartitionType).to receive(:run).and_return(:abort)
+          allow(controller).to receive(:available_partition_types).and_return(available_types)
         end
 
         let(:controller) { Y2Partitioner::Actions::Controllers::Partition.new(disk_name) }
+        let(:available_types) { Y2Storage::PartitionType.all }
 
         it "removes the filesystem" do
           expect(controller).to receive(:delete_filesystem)
@@ -251,5 +254,64 @@ def current_graph
         expect(partitions_after).to eq(partitions_before)
       end
     end
+
+    context "if some dialog is skipped and then the user goes back" do
+      let(:scenario) { "empty_hard_disk_50GiB.yml" }
+      let(:disk_name) { "/dev/sda" }
+      let(:available_types) { [Y2Storage::PartitionType.new("primary")] }
+      let(:controller) { Y2Partitioner::Actions::Controllers::Partition.new(disk_name) }
+
+      before do
+        allow(controller).to receive(:available_partition_types).and_return(available_types)
+        allow(subject).to receive(:type).and_return(:next)
+        allow(Y2Partitioner::Dialogs::PartitionSize).to receive(:run).and_return(:back)
+        allow(Y2Partitioner::Actions::Controllers::Partition).to receive(:new).with(disk_name)
+          .and_return(controller)
+        allow(subject).to receive(:skip_steps).and_call_original
+      end
+
+      it "does not run again skipped dialogs" do
+        expect(subject).to receive(:type).once.and_return(:next)
+        action.run
+      end
+
+      it "returns :back" do
+        expect(action.run).to eq :back
+      end
+    end
+  end
+
+  describe "#type" do
+    let(:scenario) { "empty_hard_disk_50GiB.yml" }
+    let(:disk_name) { "/dev/sda" }
+    let(:controller) { Y2Partitioner::Actions::Controllers::Partition.new(disk_name) }
+    let(:available_types) { Y2Storage::PartitionType.all }
+
+    before do
+      allow(controller).to receive(:available_partition_types).and_return(available_types)
+      subject.instance_variable_set(:@part_controller, controller)
+    end
+
+    context "when there is no partition types available" do
+      let(:available_types) { [] }
+      it "raises an error" do
+        expect { subject.type }.to raise_error("No partition type possible")
+      end
+    end
+
+    context "when there is only one partition type available" do
+      let(:available_types) { [Y2Storage::PartitionType.new("extended")] }
+      it "uses it as the type for the new partition" do
+        expect(controller).to receive(:type=).with(available_types.first)
+        subject.type
+      end
+    end
+
+    context "when there is more than one partition type available" do
+      it "runs the dialogs for choising the partition type" do
+        expect(Y2Partitioner::Dialogs::PartitionType).to receive(:run)
+        subject.type
+      end
+    end
   end
 end

test/y2partitioner/actions/controllers/partition_test.rb

@@ -283,4 +283,20 @@
       end
     end
   end
+
+  describe "#available_partition_types" do
+    let(:scenario) { "spaces_5_3_extended.yml" }
+    let(:disk_name) { "/dev/sda" }
+
+    it "returns a list of Y2Storage::PartitionType" do
+      partition_types = subject.available_partition_types
+      expect(partition_types).to be_a(Array)
+      expect(partition_types).to be_all(Y2Storage::PartitionType)
+    end
+
+    it "returns the possible partition types for the unused optimally aligned slots" do
+      partition_types = subject.available_partition_types
+      expect(partition_types.first).to eq(Y2Storage::PartitionType::LOGICAL)
+    end
+  end
 end

test/y2partitioner/dialogs/partition_type_test.rb

@@ -45,10 +45,11 @@
 describe Y2Partitioner::Dialogs::PartitionType::TypeChoice do
   let(:controller) do
     double(
-      "PartitionController", unused_slots: slots, unused_optimal_slots: slots, disk_name: "/dev/sda"
+      "PartitionController",
+      available_partition_types: Y2Storage::PartitionType.all,
+      disk_name:                 "/dev/sda"
     )
   end
-  let(:slots) { [double("Slot", :"possible?" => true)] }
 
   subject { described_class.new(controller) }