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.

Sign up for GitHub

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

Jump to bottom

Move to publishing api v2 #2394

Merged
merged 9 commits into from
Nov 27, 2015
Merged

Move to publishing api v2 #2394

merged 9 commits into from
Nov 27, 2015

Conversation

jamiecobbett
Copy link
Contributor

@benilovj benilovj force-pushed the move-to-publishing-api-v2 branch 4 times, most recently from d95d778 to 4106194 Compare November 25, 2015 21:39
@binaryberry binaryberry changed the title [DON'T MERGE] Move to publishing api v2 Move to publishing api v2 Nov 26, 2015
@binaryberry binaryberry changed the title Move to publishing api v2 [DON'T MERGE] Move to publishing api v2 Nov 26, 2015
@benilovj
Copy link
Contributor

I've updated the gds-api-adapters dependency to point to the new gem (26.0.0).

@jamiecobbett jamiecobbett changed the title [DON'T MERGE] Move to publishing api v2 Move to publishing api v2 Nov 27, 2015
benlovell
benlovell reviewed Nov 27, 2015
View reviewed changes
test/integration/publishing_test.rb

perform_force_publishing_for(@draft_edition)

assert_requested request
requests.each { |request| assert_requested request }
Copy link
Contributor

Choose a reason for hiding this comment

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

This occurs frequently enough that it could be extracted into a shared assertion. I looked at webmock and it wouldn't be too much of an undertaking to contribute there also, perhaps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would you call the new method?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the method lives within whitehall or gds-api-adapters, I'd suggest assert_all_requested(array). If it were contributed back to webmock, I'd just allow passing an array into assert_requested itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just as @benilovj says. 👍

@benlovell
Copy link
Contributor

Looks good other than the small issue regarding the assert_requested thing. 😍 ✨

@benilovj
Introduce a Publishing API v2 client
fa31acd 
We still need to keep the v1 client around, because publish intents
will continue to be published against the v1 client.
@benilovj
Bump gds-api-adapters gem to pick up Publishing API v2 stubs
58b1179
@benilovj
Add Publishing API v2 test helpers
03dacc6
@benilovj
Update Publishing API workers from v1 to v2 (where possible)
d71e7c7 
Publishing API v1 endpoints will be deprecated soon.
v2 requires sending the content and links to different endpoints.

Publish intents haven't been implemented in v2, so those call stay
as they are.
@benilovj
Update integration tests after Publishing API V2 migration
bc5dacb
@issyl0
Copy link
Contributor

issyl0 commented Nov 27, 2015

I was just wondering about 472e149. Here, you're using V1 and V2 in parallel. We thought, when planning this for the contacts app (contacts-admin/contacts-frontend), that the "dual publishing" migration step only applied to old old world apps, and that we didn't need to worry about dual publishing to both V1 and V2 'cause it was essentially the same thing. Is that not the case? Have we misunderstood?

@elliotcm
Copy link
Contributor

@issyl0 It looks to me like only "publish intents" are being sent to v1. These aren't supported by v2 and won't be needed later.

@issyl0
Copy link
Contributor

issyl0 commented Nov 27, 2015

@elliotcm Ahh, OK. Thanks.

@binaryberry binaryberry force-pushed the move-to-publishing-api-v2 branch 2 times, most recently from e122d2c to a0ecb5a Compare November 27, 2015 16:30
@binaryberry binaryberry force-pushed the move-to-publishing-api-v2 branch 2 times, most recently from 4c1b940 to 6ed331d Compare November 27, 2015 16:55
benilovj and others added 3 commits November 27, 2015 17:04
@benilovj
Fix functional tests after PubAPI v1 => v2 migration
b2bd72e
@jamiecobbett
Stub publishing-api calls in Cucumber specs
80c5cc3
@benilovj
Publishing API: clarify wording around saving drafts
29c8a10 
We shouldn't refer to the draft store, because that's implementation
detail. Saving the draft is what we're actually doing.
@elliotcm elliotcm mentioned this pull request Nov 27, 2015
Closed
binaryberry added a commit that referenced this pull request Nov 27, 2015
@binaryberry
Merge pull request #2394 from alphagov/move-to-publishing-api-v2
517a3d0 
Move to publishing api v2
@binaryberry binaryberry merged commit 517a3d0 into master Nov 27, 2015
@benilovj
Copy link
Contributor

🎆

@benilovj benilovj deleted the move-to-publishing-api-v2 branch January 19, 2016 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jamiecobbett @benilovj @benlovell @issyl0 @elliotcm @binaryberry