Skip to content
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

Upload Images in Moments #1444

Merged
merged 14 commits into from
Jun 25, 2019
Merged

Conversation

bmwachajr
Copy link
Contributor

@bmwachajr bmwachajr commented Apr 11, 2019

Description

When creating a moment. it is valuable to upload an image to capture the moment/mood of the user.

More Details

More Details:

  • Upload images to cloudinary.

To do:

  • Create a cloudinary service
  • create an picture upload endpoint
  • add tests
  • Ship it.

Corresponding Issue

Fixes #1480

Screenshots

Test Coverage


🚫

@julianguyen
Copy link
Member

Is this ready for review? :)

@bmwachajr
Copy link
Contributor Author

@julianguyen yes it is ready for review. A couple of things to note.

  1. Do we have the cloudinary env values set on CircleCi?
  2. Integration Test for the API endpoint has not yet been committed, I keep getting a RoutingError.

@bmwachajr bmwachajr changed the title [WIP]Upload Images in Moments Upload Images in Moments May 27, 2019
@julianguyen
Copy link
Member

julianguyen commented May 27, 2019

We set the Cloudinary env variables in Heroku!
I wonder if we need them in CircleCI for those Cloudinary related tests to pass. Did these tests pass locally for you with or without the variables? @bmwachajr

julianguyen
julianguyen previously approved these changes May 27, 2019
Copy link
Member

@julianguyen julianguyen left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Great start 🚀 Let me know if I need to set the environment variables for Cloudinary in CircleCI!

app/controllers/moments_controller.rb Outdated Show resolved Hide resolved
spec/services/cloudinary_service_spec.rb Outdated Show resolved Hide resolved
@bmwachajr
Copy link
Contributor Author

@julianguyen yeah, all tests pass locally. It would be a good idea to set the environment variables for Cloudinary in CircleCI! Otherwise, once this PR is merged we shall always have these tests failing.

@bmwachajr bmwachajr self-assigned this May 28, 2019
@bmwachajr bmwachajr force-pushed the upload-picture-to-moment branch 4 times, most recently from e3a2df0 to d2cf480 Compare June 6, 2019 13:00
julianguyen
julianguyen previously approved these changes Jun 7, 2019
@julianguyen julianguyen merged commit ddb4291 into ifmeorg:master Jun 25, 2019
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.

Back-end: uploading Images in Moments and Strategies
2 participants