-
Notifications
You must be signed in to change notification settings - Fork 12
[WIP] #17 Migrate from paperclip to active storage #137
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
Changes from all commits
3b06bd0
fd85f1f
12c1500
03b8632
d25e803
9ba7714
142bfad
435d504
c36870f
f8ac938
c0011a1
285f004
cf15487
67bde84
ab44db3
db9d047
b92bf76
4ac2603
2bd31c6
a9c036d
0791a8e
866d5d3
00d9dc0
5fb0b7f
61a751b
f497e02
ddf6ada
518a870
d3a05d2
770177f
7ebcdee
2c0ea41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,3 +48,5 @@ node_modules | |
| *.local | ||
|
|
||
| .vite | ||
| # Ignore local Active Storage | ||
| storage | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| class Attachment < ApplicationRecord | ||
| belongs_to :owner, polymorphic: true | ||
|
|
||
| has_attached_file :file | ||
| validates_attachment :file, content_type: { content_type: %w(application/pdf application/msword image/gif image/jpg image/jpeg image/png) } | ||
| ACCEPTED_CONTENT_TYPES = %w[application/pdf application/msword image/gif image/jpeg image/png].freeze | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might we want to expand the list? here's what i ended up using in another application to support additional document spreadsheet types and avoid upload errors users were having. for workshops, there's a need to support uploading a video, so i added video types in here too UPLOAD_TYPES = [ Documents'application/pdf', Videos'video/mp4', Images (regex)/\Aimage/.*\Z/
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That definitely makes sense! I was trying to keep this PR as close to a one for one change, at least until all the files are migrated to avoid any confusion on what actually needs to be changes vs. improvements. Do you think we could open a draft issue for your suggested changes until the migration is complete?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. YES, love this separation of concerns |
||
| has_one_attached :file | ||
| validates :file, content_type: ACCEPTED_CONTENT_TYPES | ||
|
|
||
| def name | ||
| 'Pdf Attachment' | ||
| "Pdf Attachment" | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,7 @@ class Image < ApplicationRecord | |
| belongs_to :owner, polymorphic: true | ||
| belongs_to :report, optional: true | ||
|
|
||
| has_attached_file :file, | ||
| styles: { medium: '300x300>', thumb: '100x100>' }, | ||
| default_url: | ||
| ActionController::Base.helpers.asset_path( | ||
| 'workshop_default.png' | ||
| ) | ||
| validates_attachment :file, content_type: { content_type: ["image/jpg", "image/jpeg", "image/png", "image/gif"] } | ||
| ACCEPTED_CONTENT_TYPES = ["image/jpeg", "image/png", "image/gif"].freeze | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment -- how about this instead?
|
||
| has_one_attached :file | ||
| validates :file, content_type: ACCEPTED_CONTENT_TYPES | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,11 @@ class MediaFile < ApplicationRecord | |
| belongs_to :workshop, optional: true | ||
| belongs_to :workshop_log, optional: true | ||
|
|
||
| has_attached_file :file | ||
|
|
||
| FORM_FILE_CONTENT_TYPES = ["image/jpeg", "image/jpg", "image/png", | ||
| "application/pdf", "application/msword", | ||
| "application/vnd.openxmlformats-officedocument.wordprocessingml.document", | ||
| "application/vnd.ms-excel", | ||
| "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"] | ||
|
|
||
| validates_attachment :file, content_type: { content_type: FORM_FILE_CONTENT_TYPES } | ||
| FORM_FILE_CONTENT_TYPES = ["image/jpeg", "image/png", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you remove
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! |
||
| "application/pdf", "application/msword", | ||
| "application/vnd.openxmlformats-officedocument.wordprocessingml.document", | ||
| "application/vnd.ms-excel", | ||
| "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"] | ||
| has_one_attached :file | ||
| validates :file, content_type: FORM_FILE_CONTENT_TYPES | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ class ProjectUser < ApplicationRecord | |
| belongs_to :project | ||
| belongs_to :user | ||
|
|
||
| scope :liaisons, -> { where(position: 1) } | ||
| scope :liaisons, -> { where(position: "liaison") } | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was changed for readability with @marc during debugging an enum issue with the rails 8.1 upgrade. |
||
| # Validations | ||
| validates_presence_of :project_id | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These method should probably be consolidated once the storage attachments have been migrated. The stakeholders only want to upload a single file that can display correctly in different places.
legacyshould be removed as it was used from a previous data transfer but we need to check what files are actually affected by that.