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

Add price to service templates. #367

Merged
merged 1 commit into from
May 14, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions db/migrate/20190513155251_add_price_to_service_templates.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddPriceToServiceTemplates < ActiveRecord::Migration[5.0]
def change
add_reference :service_templates, :currency, :type => :bigint
Copy link
Contributor

@lpichler lpichler May 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have chargeback_rate_detail_currency here - to see that it is related to chargeback_rate_detail_currencies table.

add_reference :service_templates, :chargeback_rate_detail_currency, :type => :bigint

you can specify foreign key in the model.

@carbonin @lfu what do you think ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea is to make the method #currency and the column currency_id so the relationship in the model specifies the class https://github.com/ManageIQ/manageiq/pull/18754/files#diff-9257fff45a7d1258305c9a6b7d15e322R66

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate that chargeback_rate_detail_currency isn't named just currencies because it doesn't look like the table has anything chargeback specific in it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carbonin Exactly what I am going to say.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the effort to rename the table :)

add_column :service_templates, :price, :float
end
end