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.

Sign up for GitHub

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

[WIP] Add type column to service_order table #328

Closed
wants to merge 3 commits into from
Closed

[WIP] Add type column to service_order table #328

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19bb4ce
Add type column to service_order table.
Jan 23, 2019
9ff7413
Change column from type to request_type, and use joined string as res…
Mar 19, 2019
da0823d
Switch back to STI.
djberg96 Jan 27, 2020
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
15 changes: 15 additions & 0 deletions db/migrate/20190123145942_add_type_column_to_service_order.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class AddTypeColumnToServiceOrder < ActiveRecord::Migration[5.0]
def up
add_column :service_order, :type, :string

say_with_time("Set service_order type") do
ServiceOrder.all.each do |service_order|
service_order.update_attribute(:type, service_order.miq_requests.pluck(:type))
Copy link
Member

Choose a reason for hiding this comment

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

So 2 things

  1. Type is reserved for STI to just taking the type from MiqRequest is going to be an issue. If you're going to set a :type column it should match a sub-class of ServiceOrder so something like if MiqRequest is ServiceTemplateProvisionRequest then ServiceOrder would become ServiceProvisionOrder and ServiceTemplateTransformationPlanRequest => ServiceTransformationOrder

  2. Updating for every service_order isn't very efficient, it'd be much better to group_by miq_request type and do an update_all

Copy link
Contributor

@himdel himdel Jan 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

service_order.miq_requests.pluck(:type)

That yields an array of all the types, right? (When there are multiple requests.)

Possibly something like

types = service_order.miq_requests.pluck(:type).sort.uniq
service_order.update_attribute(:type, types.first) if types.count == 1

?

Copy link
Contributor

@himdel himdel Jan 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think..

SELECT string_agg(DISTINCT type, ', ') AS type, service_order_id
FROM miq_requests
WHERE service_order_id IS NOT NULL
GROUP BY service_order_id;

gives you the right list, so you can transform it into a single update, and it should mostly work.
(No idea how to say that in rails-speak though :).)

We may want to clear any types with a comma after though. (And I guess not use type but something like request_type?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agrare Ok, we don't want STI here, so I've changed the column to request_type as per @himdel's suggestion. It's now a sorted and comma-joined string in the event there is more than one request type.

If there's a more efficient way of doing this update I think I will need a little help putting it together.

end
end
end

def down
remove_column :service_order, :type
end
end