Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Settings.product.transformation #236

Merged
merged 1 commit into from
Jul 24, 2018
Merged

Remove Settings.product.transformation #236

merged 1 commit into from
Jul 24, 2018

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Jul 18, 2018

v2v product feature, this should be always on from 5.9.3 on - replaced by RBAC.

Goes together with ManageIQ/manageiq#17711 (and ManageIQ/manageiq-v2v#490).

Cc @Fryguy

@himdel
Copy link
Contributor Author

himdel commented Jul 18, 2018

@miq-bot add_label transformation, gaprindashvili/yes

@gmcculloug
Copy link
Member

@miq-bot assign @Fryguy

@gmcculloug
Copy link
Member

@himdel We do not back-port migrations so the gaprindashvili/yes label is not needed here. This would be applied to the next release, Hammer.

@himdel
Copy link
Contributor Author

himdel commented Jul 18, 2018

OK, removing the label, thanks :)

@miq-bot remove_label gaprindashvili/yes

@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2018

@himdel unrecognized command 'remove', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

v2v product feature, this should be always on from 5.9.3 on
@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2018

Checked commit https://github.com/himdel/manageiq-schema/commit/0f14276be08e50b76d3b4f2736de8f52411bde6e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@himdel
Copy link
Contributor Author

himdel commented Jul 20, 2018

@miq-bot add_label gaprindashvili/no
@miq-bot remove_label gaprindashvili/yes

@Fryguy Fryguy merged commit 95166f3 into ManageIQ:master Jul 24, 2018
@Fryguy Fryguy added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 24, 2018
@Fryguy Fryguy added the data label Jul 24, 2018
@himdel himdel deleted the product-trans branch July 24, 2018 15:50
@Fryguy
Copy link
Member

Fryguy commented Aug 9, 2018

@himdel I totally missed that this was a data migration that doesn't have any specs...can you add a spec in a separate PR please?

@himdel
Copy link
Contributor Author

himdel commented Aug 9, 2018

I ...am trying :).

Do we have any docs on running specs here?

$ be rspec spec/migrations/20180718132840_remove_transformation_product_setting_spec.rb
FF

Failures:

  1) RemoveTransformationProductSetting#up with empty tables
     Failure/Error: suppress_migration_messages { ActiveRecord::Migrator.migrate('db/migrate', version) }
     
     ActiveRecord::NoDatabaseError:
       FATAL:  database "dummy_test" does not exist
     # ./spec/support/migration_helper.rb:89:in `block in migrate_to'
     # ./spec/support/migration_helper.rb:83:in `suppress_migration_messages'
     # ./spec/support/migration_helper.rb:89:in `migrate_to'
     # ./spec/support/migration_helper.rb:25:in `prepare_migrate'
     # ./spec/support/migration_helper.rb:8:in `block (2 levels) in migration_context'
     # ------------------
     # --- Caused by: ---
     # PG::ConnectionBad:
     #   FATAL:  database "dummy_test" does not exist
     #   ./spec/support/migration_helper.rb:89:in `block in migrate_to'

  2) RemoveTransformationProductSetting#up removes the /product/transformation key
     Failure/Error: suppress_migration_messages { ActiveRecord::Migrator.migrate('db/migrate', version) }
     
     ActiveRecord::NoDatabaseError:
       FATAL:  database "dummy_test" does not exist
     # ./spec/support/migration_helper.rb:89:in `block in migrate_to'
     # ./spec/support/migration_helper.rb:83:in `suppress_migration_messages'
     # ./spec/support/migration_helper.rb:89:in `migrate_to'
     # ./spec/support/migration_helper.rb:25:in `prepare_migrate'
     # ./spec/support/migration_helper.rb:8:in `block (2 levels) in migration_context'
     # ------------------
     # --- Caused by: ---
     # PG::ConnectionBad:
     #   FATAL:  database "dummy_test" does not exist
     #   ./spec/support/migration_helper.rb:89:in `block in migrate_to'

Finished in 0.01529 seconds (files took 0.85142 seconds to load)
2 examples, 2 failures

@himdel
Copy link
Contributor Author

himdel commented Aug 9, 2018

diff --git a/spec/migrations/20180718132840_remove_transformation_product_setting_spec.rb b/spec/migrations/20180718132840_remove_transformation_product_setting_spec.rb
new file mode 100644
index 0000000..72e00ac
--- /dev/null
+++ b/spec/migrations/20180718132840_remove_transformation_product_setting_spec.rb
@@ -0,0 +1,17 @@
+require_migration
+
+describe RemoveTransformationProductSetting do
+  let(:settings_change_stub) { migration_stub(:SettingsChange) }
+
+  migration_context :up do
+    it "removes the /product/transformation key" do
+      setting_changed = settings_change_stub.create!(:key => "/product/transformation", :value => true)
+      setting_ignored = settings_change_stub.create!(:key => "/product/magic", :value => true)
+
+      migrate
+
+      expect(setting_changed.reload.value).to eq(nil) # TODO?
+      expect(setting_ignored.reload.value).to eq(true)
+    end
+  end
+end

This is probably close, I'll create a PR once I can test it

@carbonin
Copy link
Member

carbonin commented Aug 9, 2018

@himdel bundle exec rake spec:setup should do the trick

Just make sure you set up the spec/dummy/config/database.yml so that it can connect to your local db.

@himdel
Copy link
Contributor Author

himdel commented Aug 9, 2018

Aah, silly me, thanks! :) Good to know about spec/dummy/config/database.yml (but turns out it works out of the box)

@himdel
Copy link
Contributor Author

himdel commented Aug 9, 2018

Should be ready in #249 :)

@Fryguy Fryguy added v2v and removed transformation labels Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants