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

Rebuild API service as standard Rails app #36

merged 18 commits into from Oct 29, 2014

Rebuild API service as standard Rails app #36

merged 18 commits into from Oct 29, 2014


Copy link

Replaces #34 - this is a rebase/reorder of the work in that branch to be better about preserving history.

While hexagonal architecture is fine in principle, the complexity of the abstraction was getting in the way of delivering important bugs/features, namely:

This pull request, almost entirely by @elliotcm, rewrites the application as a standard Rails application, and as a bonus implements queuing using Sidekiq and fixes the topic bug.

Note: this will require merging a separate PR in gds-api-adapters to fix the URL namespacing that's changed (from underscore separated to dash-separated), and careful deployment (we'll want to wipe the database before deploying, as the migrations and schema have changed).

I recommend checking out the code and running it to test this - reading the diff is going to be hairy.

@bradwright bradwright force-pushed the rebuild-rebased branch 2 times, most recently from c04115d to d57dd82 Compare October 28, 2014 22:04
Paired with @rboulton.

We now check that we have all the keys rather than an overlapping subset
of them, and use this in `where_tags_equal`.

Also add passing tests of
SubscriberList.with_at_least_one_tag_of_each_type to narrow down its
behaviour a bit more, and tests of SubscriberList.where_tags_equal,
which fail in some cases.
james added a commit that referenced this pull request Oct 29, 2014
Rebuild API service as standard Rails app
@james james merged commit 22fbe97 into master Oct 29, 2014
@bradwright bradwright deleted the rebuild-rebased branch October 31, 2014 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants