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

Remove deprecated pre-TGM Contentful lib functions #1022

Merged
merged 3 commits into from
Mar 27, 2018

Conversation

aaronschachter
Copy link
Contributor

@aaronschachter aaronschachter commented Mar 27, 2018

What's this PR do?

  • Removes lib/contentful functions no longer used in production: fetchBroadcast,fetchCampaign, fetchMessageForCampaignId, and renderMessageForPhoenixCampaign
  • Renames parseDefaultAndOverrideContentfulCampaignsResponse as parseDefaultAndOverrideCampaignsResponse, and adds to exports for test coverage
  • Renames fetchDefaultAndOverrideContentfulCampaignsForCampaignId as fetchDefaultAndOverrideCampaignsForCampaignId, adds test coverage for the middleware that calls it (lib/middleware/campaigns-single/contentful-campaigns)

How should this be reviewed?

Verify GET /campaigns and GET /campaigns/:id responses return expected data.

Any background context you want to provide?

  • I'd like to deprecate the use of the Contentful campaign with campaignId set to default for defining default fields, and instead hardcode the default values to use if an optional field doesn't exist on the Campaign. Didn't make that change in this branch to help keep it reviewable.

  • I'll be testing on staging by setting the Phoenix URL config variable to point to Ashes, putting Support Ashes and Phoenix URLs via config  #1019 on hold.

Relevant tickets

Cleanup work to prep for new Gambini types and fields in #1021

Checklist

  • Tested on staging.

@aaronschachter aaronschachter temporarily deployed to ds-mdata-responder-staging March 27, 2018 03:27 Inactive
@aaronschachter
Copy link
Contributor Author

I've set the Phoenix API config var back to Ashes, tested this on staging, and verified all API endpoints are working as expected.

Copy link
Contributor

@rapala61 rapala61 left a comment

Choose a reason for hiding this comment

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

👍

@aaronschachter aaronschachter merged commit 278d385 into master Mar 27, 2018
@aaronschachter aaronschachter deleted the cleanup/contentful branch March 27, 2018 17:57
@aaronschachter aaronschachter mentioned this pull request Mar 29, 2018
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants