Skip to content

Conversation

@toastercup
Copy link
Member

@toastercup toastercup commented Apr 24, 2017

Purpose

This PR 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 (Amazon S3 > cb-talent-development-cortex-dev > asset)
    • 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#488

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

@@ -0,0 +1,12 @@
.thumbnail-placeholder {
background-color: #E4E4E4; // TODO: determine how to inherit variables from Cortex. This should be $color-grey-evenlighter
Copy link
Contributor

Choose a reason for hiding this comment

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

A separate style library?

Copy link
Member Author

Choose a reason for hiding this comment

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

@arelia That'd do it! It was late that night.. We should get started on that ASAP for V3.

@arelia
Copy link
Contributor

arelia commented Apr 26, 2017

I can successfully edit an asset and replace it with a different file type and update title, but I get a "Title Title Must Be Unique" error message

The original file was a PDF with a title like "Hiring Dot Nav".
screen shot 2017-04-26 at 10 17 21

It appears to have successfully been replaced with the space image and the "Space is Unique" title
image

@toastercup
Copy link
Member Author

@arelia I noticed this as well, and actually added a comment to our code that this 'text uniqueness' feature does not work when you're updating an existing ContentItem's title. This was a pre-existing bug and is being documented in JIRA. Thanks for being thorough, though!

@@ -0,0 +1,12 @@
.thumbnail-placeholder {
background-color: #E4E4E4; // TODO: Abstract to cortex-style-base library. This should be $color-grey-evenlighter
Copy link
Contributor

Choose a reason for hiding this comment

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

Even before the style base for Cortex is founded could you please extract this so we can just copy and paste the variables when the time comes

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure - I'll just copy all variables from cortex to cortex-plugins-core, with the expectation that these need to be removed once cortex-plugins-core can pull from a cortex-style-base. Thanks!

@ElliottAYoung
Copy link
Contributor

@toastercup Should we expedite the title bug @arelia mentioned to the next sprint, it seems like something that should be addressed before V2 is released

@toastercup
Copy link
Member Author

@ElliottAYoung Yes - there's actually several bugs I uncovered that I'm going to push to be addressed next sprint

@toastercup toastercup force-pushed the topic/COR-704-Rebuild-AssetFieldType branch from 5385eb9 to 52f03cc Compare April 26, 2017 16:11
@toastercup toastercup merged commit 620749f into develop Apr 26, 2017
@toastercup toastercup deleted the topic/COR-704-Rebuild-AssetFieldType branch April 26, 2017 17:15
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