Service template picture #18689
Service template picture #18689
Conversation
d-m-u
force-pushed
the
service_template_picture
branch
4 times, most recently
from
eef4819
to
3381b26
Compare
| if value | ||
| super unless value.kind_of?(Hash) | ||
|
|
||
| super(create_picture(value)) |
There was a problem hiding this comment.
How does this instance method get to the class method?
There was a problem hiding this comment.
As such, are there specs for this?
There was a problem hiding this comment.
This's tested! It's in this PR and also at https://github.com/ManageIQ/manageiq/pull/18674/files#diff-7338735460ffd6eebe447a430abdec8eR1051. Beyond that I'm testing Rails since the method is here: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/builder/singular_association.rb#L32
There was a problem hiding this comment.
Looks like it's calling the dynamic method for the singular association... My only concern is if self hasn't been persisted yet. I don't know if create_picture would require it be saved first. I don't know. You might have to call Picture.new or something like that and pass that to super. 🤷♂
There was a problem hiding this comment.
irb(main):007:0* st = ServiceTemplate.create!(:name => "blah")
(0.2ms) BEGIN
User Load (0.4ms) SELECT "users".* FROM "users" WHERE ("users"."id" BETWEEN $1 AND $2) AND "users"."userid" IS NULL LIMIT $3 [["id", 0], ["id", 999999999999], ["LIMIT", 1]]
User Inst Including Associations (0.0ms - 0rows)
SQL (6.2ms) INSERT INTO "service_templates" ("name", "guid", "created_at", "updated_at", "miq_group_id", "tenant_id", "internal", "service_type", "generic_subtype") VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) RETURNING "id" [["name", "blah"], ["guid", "cb823fc0-8b3f-40a3-b195-ee71866d7539"], ["created_at", "2019-04-29 19:26:38.584945"], ["updated_at", "2019-04-29 19:26:38.584945"], ["miq_group_id", 1], ["tenant_id", 1], ["internal", "f"], ["service_type", "atomic"], ["generic_subtype", nil]]
(0.4ms) COMMIT
=> #<ServiceTemplate id: 44, name: "blah", description: nil, guid: "cb823fc0-8b3f-40a3-b195-ee71866d7539", type: nil, service_template_id: nil, options: {}, created_at: "2019-04-29 19:26:38", updated_at: "2019-04-29 19:26:38", display: nil, evm_owner_id: nil, miq_group_id: 1, service_type: "atomic", prov_type: nil, provision_cost: nil, service_template_catalog_id: nil, long_description: nil, tenant_id: 1, generic_subtype: nil, deleted_on: nil, internal: false, zone_id: nil>
irb(main):009:0> st.create_picture(:content => "iVBORw0KGgoAAAAN", :extension => "jpg")
(0.2ms) BEGIN
SQL (3.6ms) INSERT INTO "pictures" ("resource_id", "resource_type", "content", "extension", "md5") VALUES ($1, $2, $3, $4, $5) RETURNING "id" [["resource_id", 44], ["resource_type", "ServiceTemplate"], ["content", "<16 bytes of binary data>"], ["extension", "jpg"], ["md5", "4617a55a374175d8d7479071f41db6c7"]]
(0.4ms) COMMIT
Picture Load (1.0ms) SELECT "pictures".* FROM "pictures" WHERE "pictures"."resource_id" = $1 AND "pictures"."resource_type" = $2 LIMIT $3 [["resource_id", 44], ["resource_type", "ServiceTemplate"], ["LIMIT", 1]]
Picture Inst Including Associations (0.1ms - 1rows)
=> #<Picture id: 15, resource_id: 44, resource_type: "ServiceTemplate", content: "iVBORw0KGgoAAAAN", extension: "jpg", md5: "4617a55a374175d8d7479071f41db6c7">
Kewl!
👍
There was a problem hiding this comment.
For what it is worth, this fails if the picture exists first:
picture = Picture.create(:content => "foo", :extension => "jpg")
ServiceTemplate.create(:name => "foo", :picture => picture)Currently used like this in specs:
d-m-u
force-pushed
the
service_template_picture
branch
3 times, most recently
from
aa0f0d1
to
94b2f91
Compare
| @@ -403,6 +399,14 @@ def provision_request(user, options = nil, request_options = {}) | |||
| result[:request] | |||
| end | |||
|
|
|||
| def picture=(value) | |||
| if value | |||
| super unless value.kind_of?(Hash) | |||
There was a problem hiding this comment.
I like how we can choose which format we want to use and we can deprecate the other easily. The problem with rails create_picture magic is it makes passing around hashes easier to deal with. I think we still want picture= to accept a Picture object in the future, but this can be a bridge to doing that.
d-m-u
force-pushed
the
service_template_picture
branch
from
94b2f91
to
ee35349
Compare
d-m-u
force-pushed
the
service_template_picture
branch
from
ee35349
to
c617cbc
Compare
|
Checked commits d-m-u/manageiq@75d8e0b~...c617cbc with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Service template picture (cherry picked from commit 5f37ae7) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1702479
|
Hammer backport details: |
We're not updating service templates with pictures correctly (or updating them) because the template is expecting a picture and instead gets a hash of options to create the picture. (Sorry, LJ! I'm a doofus and https://github.com/ManageIQ/manageiq/pull/18674/files#diff-9257fff45a7d1258305c9a6b7d15e322R519 doesn't actually update the picture, funnily enough.)
It's the actual working update code for #18674.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1683723