Skip to content

Conversation

@toastercup
Copy link
Member

@toastercup toastercup commented Apr 26, 2017

Purpose

This PR features:

  • Supporting functionality for the Shrine-backed AssetFieldType rebuild
  • Comments denoting targets for refactor
  • Style fixes and improvements

This rebuild features:

  • Asset Updating
  • Thumbnail placeholders for non-image assets
  • Accepted/Supported File Types feature for file input field
  • Removes media_title functionality for dynamic path/location. This will be discussed with Justin + team. This feature introduced tech debt with little benefit, and also has pre-existing issues with asset updating.

JIRA

https://cb-content-enablement.atlassian.net/browse/COR-704

Steps to Take On Prod

  • Reseed ContentTypes

Changes

  • Changes to setup

    • .env changes:
      • HOST is now FOG_HOST, to be more explicit
      • New: HOST_ALIAS is used during ContentType seeding to configure the CDN for Media Assets
      • New: S3_ACCESS_KEY_ID is self-explanatory, and is only for Beta's AssetFieldType. It was created as a replacement for AWS_ACCESS_KEY_ID, whose naming is too broad for what it's intended for.
      • New: S3_SECRET_ACCESS_KEY is self-explanatory, and is only for Beta's AssetFieldType for the same reasons as S3_ACCESS_KEY_ID.
      • The old AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY continue to be used for Legacy
      • Variables like S3_BUCKET_NAME are used both for Legacy and Beta. This can be changed in the future if need be, but right now they share the same credentials.
  • Architectural changes

    • Lots!
  • Migrations

    • N/A
  • Library changes

    • cortex-plugins-core needs to be bumped prior to merging
  • Side effects

    • N/A

Screenshots

  • Before
    N/A

  • After
    N/A

QA Links

http://web.cortex-1.development.c66.me/ (admin@cortexcms.org/welcome1)

How to Verify These Changes

  • Specific pages to visit

    • Media
  • Steps to take

    • Try various CRUD-y things:
      • Create Media using various filetypes: image + non-image
      • Make sure you can update/overwrite existing Media with a new Asset
      • Try replacing an existing asset with a file of a different type (i.e. PNG -> PDF)
    • Ensure asset is uploaded to S3
    • Ensure asset link is generated using CDN
    • Ensure thumbnails/versions are generated and uploaded correctly
    • Ensure Media can be referred to by Blog (WYSIWYG Media insertion + Featured Media selection)
  • Responsive considerations

    • N/A

Relevant PRs/Dependencies

cortex-cms/cortex-plugins-core#57

Additional Information

  • We should discuss threading
  • This PR implements almost 1:1 feature parity with our Paperclip implementation, but there are several gaps to discuss

…ld, comments denoting targets for refactor, style fixes and improvements
@toastercup toastercup force-pushed the topic/COR-704-Rebuild-AssetFieldType branch from 179a49d to 4ffb047 Compare April 26, 2017 16:07
@toastercup toastercup force-pushed the topic/COR-704-Rebuild-AssetFieldType branch from 4ffb047 to 441707b Compare April 26, 2017 16:12
@toastercup toastercup merged commit 8e0fde3 into develop Apr 26, 2017
@toastercup toastercup deleted the topic/COR-704-Rebuild-AssetFieldType branch April 26, 2017 17:18
@MKwenhua
Copy link
Contributor

Although this has already been merged, I would like to leave a comment on why moving to Shrine was the right choice despite the extra work needed to implement.

Currently our FieldTypes come through externally via our plugins library meaning it's more decoupled from Rails than it would be if it was hardcoded in Cortex itself. And since extensibility/flexibility is one of the primary goals of Cortex an external plugin system is necessary, which means Paperclip just wont work.

Few points on why Paperclip wont work:

  • Paperclip was built with lots of implicit assumptions about your applications structure and environment and will break when any of it's implicit dependencies is not there. Additionally as Rails changes internally so too will Paperclip's internal dependencies meaning any update to later Rails versions may introduce new breaking changes.
  • Black box magic does not play well with applications like Cortex which is designed to be highly composable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants