Merge pull request #705 from ancorgs/fix_1055756_sle15

Fix the check about a manual staging (bsc#1055756)

package/yast2-storage-ng.changes

@@ -1,3 +1,11 @@
+-------------------------------------------------------------------
+Fri Aug 10 11:56:40 UTC 2018 - ancor@suse.com
+
+- Fixed the warning about overwriting a manually edited partition
+  layout. Now it works even after going back and forth in the
+  installer steps (bsc#1055756).
+- 4.0.204
+
 -------------------------------------------------------------------
 Thu Aug  9 15:43:17 UTC 2018 - ancor@suse.com
 

package/yast2-storage-ng.spec

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

src/lib/y2storage/clients/inst_disk_proposal.rb

@@ -45,8 +45,6 @@ class InstDiskProposal
       include InstDialogMixin
       include PartitioningFeatures
 
-      attr_reader :manual_changed # Settings has been changed by user
-
       def initialize
         textdomain "storage"
 
@@ -62,7 +60,6 @@ def initialize
 
       def run
         log.info("BEGIN of inst_disk_proposal")
-        @manual_changed = false
 
         until [:back, :next, :abort].include?(@result)
           dialog = Dialogs::Proposal.new(@proposal, @devicegraph, excluded_buttons: excluded_buttons)
@@ -91,16 +88,14 @@ def run
       # @return [Integer]
       attr_reader :initial_staging_revision
 
-      # The user has changed partition settings.
+      # The user has changed partition settings using the Expert Partitioner.
       # Asking if these changes can be overwritten.
       def overwrite_manual_settings?
-        return true unless manual_changed
         ret = Popup.YesNo(_(
                             "Computing this proposal will overwrite manual changes \n"\
                             "done so far. Continue with computing proposal?"
         ))
         log.info "overwrite_manual_settings? return #{ret}"
-        @manual_changed = false if ret # reset for next change
         ret
       end
 
@@ -116,7 +111,8 @@ def save_to_storage_manager
       end
 
       def guided_setup
-        return unless overwrite_manual_settings?
+        return if manual_partitioning? && !overwrite_manual_settings?
+
         settings = @proposal ? @proposal.settings : new_settings
         dialog = Dialogs::GuidedSetup.new(settings, probed_analyzer)
         case dialog.run
@@ -134,7 +130,6 @@ def expert_partitioner(initial_graph)
         dialog_result = without_title_on_left { dialog.run }
 
         actions_after_partitioner(dialog.device_graph, dialog_result)
-        @manual_changed = true if @result != :abort
       end
 
       # Actions to perform after running the Partitioner
@@ -266,6 +261,13 @@ def partitioner_warning
 
         Yast2::Popup.show(message, headline: :warning, buttons: :continue_cancel, focus: :cancel)
       end
+
+      # Whether the current devicegraph was configured using the Expert Partitioner
+      #
+      # @return [Boolean] false if staging was calculated using a proposal
+      def manual_partitioning?
+        @proposal.nil?
+      end
     end
   end
 end

test/y2storage/clients/inst_disk_proposal_test.rb

@@ -28,6 +28,7 @@
 
   describe "#run" do
     let(:proposal_dialog) { double("Y2Storage::Dialogs::Proposal") }
+    let(:guided_dialog) { double("Y2Storage::Dialogs::GuidedSetup") }
     let(:storage_manager) { Y2Storage::StorageManager.instance }
 
     before do
@@ -253,6 +254,70 @@
       end
     end
 
+    describe "if the guided setup button is pressed in the proposal dialog" do
+      before do
+        allow(Y2Storage::Dialogs::Proposal).to receive(:new).and_return(proposal_dialog)
+        # First try to open the guided setup, then force quit to end the test
+        allow(proposal_dialog).to receive(:run).and_return(:guided, :abort)
+
+        allow(initial_proposal).to receive(:settings).and_return proposal_settings
+      end
+
+      let(:proposal_settings) { double("Y2Storage::ProposalSettings") }
+
+      context "when the staging devicegraph has been manually set" do
+        before { allow(proposal_dialog).to receive(:proposal).and_return nil }
+
+        it "ask the user for confirmation" do
+          expect(Yast::Popup).to receive(:YesNo)
+          client.run
+        end
+
+        context "and the user confirms to continue" do
+          before { allow(Yast::Popup).to receive(:YesNo).and_return true }
+
+          it "opens the guided setup dialog" do
+            expect(Y2Storage::Dialogs::GuidedSetup).to receive(:new).and_return(guided_dialog)
+            expect(guided_dialog).to receive(:run)
+
+            client.run
+          end
+        end
+
+        context "and the user denies the confirmation" do
+          before { allow(Yast::Popup).to receive(:YesNo).and_return false }
+
+          it "does not open the guided setup dialog" do
+            expect(Y2Storage::Dialogs::GuidedSetup).to_not receive(:new)
+
+            client.run
+          end
+        end
+      end
+
+      context "when the staging devicegraph has been set by a proposal" do
+        before do
+          allow(proposal_dialog).to receive(:proposal).and_return initial_proposal
+
+          allow(Y2Storage::Dialogs::GuidedSetup).to receive(:new).and_return(guided_dialog)
+        end
+
+        it "does not ask the user for confirmation" do
+          allow(guided_dialog).to receive(:run)
+
+          expect(Yast::Popup).to_not receive(:YesNo)
+          client.run
+        end
+
+        it "opens the guided setup dialog" do
+          expect(Y2Storage::Dialogs::GuidedSetup).to receive(:new)
+          expect(guided_dialog).to receive(:run)
+
+          client.run
+        end
+      end
+    end
+
     describe "calling the expert partitioner" do
       let(:partitioner) { double("Y2Partitioner::Dialogs::Main") }
 
@@ -418,7 +483,6 @@
     end
 
     context "processing the guided setup result" do
-      let(:guided_dialog) { double("Y2Storage::Dialogs::GuidedSetup") }
       let(:devicegraph) { double("Y2Storage::Devicegraph") }
       let(:settings) { double("Storage::ProposalSettings") }
       let(:proposal) { double("Y2Storage::GuidedProposal", devices: devicegraph, settings: settings) }
@@ -429,6 +493,9 @@
         allow(Y2Storage::Dialogs::GuidedSetup).to receive(:new).and_return(guided_dialog)
         # Just to quit
         allow(second_proposal_dialog).to receive(:run).and_return :abort
+
+        # Just to make sure the popup about overwriting a manual setup is not raised
+        allow(proposal_dialog).to receive(:proposal).and_return(proposal)
       end
 
       context "if the guided setup returns :abort" do
@@ -444,7 +511,6 @@
 
       context "if the guided setup returns :back" do
         before do
-          allow(proposal_dialog).to receive(:proposal).and_return(proposal)
           allow(proposal_dialog).to receive(:devicegraph).and_return(devicegraph)
           allow(guided_dialog).to receive(:run).and_return :back
         end
@@ -510,10 +576,6 @@
         it "aborts" do
           expect(client.run).to eq :abort
         end
-        it "recognizes that the user has NOT changed settings" do
-          client.run
-          expect(client.manual_changed).to eq(false)
-        end
       end
 
       context "if the expert partitioner returns :back" do
@@ -568,15 +630,6 @@
             .and_return(second_proposal_dialog)
           client.run
         end
-        it "recognizes that the user has changed settings" do
-          allow(Y2Storage::Dialogs::Proposal).to receive(:new).once
-            .and_return(proposal_dialog)
-          allow(Y2Storage::Dialogs::Proposal).to receive(:new).once
-            .with(nil, new_devicegraph, anything)
-            .and_return(second_proposal_dialog)
-          client.run
-          expect(client.manual_changed).to eq(true)
-        end
       end
     end
   end