Remove tenant custom icons and paperclip #13796
Conversation
spec/models/tenant_spec.rb
Outdated
| it "nulls out blank subdomain" do | ||
| expect(described_class.create!(:subdomain => " ", :parent => root_tenant).domain).to be_nil | ||
| end | ||
| end |
There was a problem hiding this comment.
I think @kbrock got a little too zealous to delete... 🔥 😮 I believe this nil_blanks is still needed. It doesn't seem related to the logo.
updated migration to create fields instead of using shortcut These are not used.
|
Checked commit kbrock@1742cea with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
kbrock
changed the title
kbrock
changed the title
| t.string :login_logo_file_name | ||
| t.string :login_logo_content_type | ||
| t.integer :login_logo_file_size | ||
| t.datetime :login_logo_updated_at |
There was a problem hiding this comment.
yeah, I'm confused, are we dropping the logo feature? I'd assume we'd have a new migration that drops the columns. Can you help me understand @kbrock ?
There was a problem hiding this comment.
What will happen to users running rake db:migrate?
New users: Nothing, and things will work
Current users (migrated): Nothing, and that's the problem (database not updated, code breaks)
There was a problem hiding this comment.
problem: helper attachment is not available.
solution: manually create the fields that were created by the attachment helper.
result: schema.rb (and database) should be identical.
check: the schema.yaml should freak if these values are different.
In the future, I'd like to remove these values, but the purpose of this change is not to change the db but rather reduce dependencies.
There was a problem hiding this comment.
I'm not sure how what you stated applies to this PR where you are changing an old migration. You literally cannot change an old migration. If you are adding a new one to undo stuff, then sure...I don't think anyone is against that.
There was a problem hiding this comment.
ohhhhhh waiiiiit....
Are you saying the attachment helper creates all of that stuff under the covers?
There was a problem hiding this comment.
@Fryguy yea - that was my intent. maybe I messed it up
There was a problem hiding this comment.
I used the existing schema.rb to give me the list of fields that were added by attachment (and the order)
There was a problem hiding this comment.
No, you just didn't make it clear...
the attachment method comes from paperclip gem and creates all of the exact same columns in this PR, so the PR is not changing anything except to no longer use the magic method from paperclip. In that case, this is fine 👍
|
removed |
|
All concerns around the migration change have been addressed since this isn't really changing behavior(schema.yml is unchanged), just removing the need for paperclip by doing what it did manually. Merging. |
Blocked: ManageIQ/manageiq-ui-classic#319
Tenancy used to be about branding.
But we didn't come up with a good solution for multi appliance storage of tenant icons.
And priorities changed.
So this feature with
Tenant#iconwas never used.And this code just became cruft
Ref: related to #13792 - but fixes a different failure