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

Add REST API to retrieve financial transactions #423

Closed
wants to merge 1 commit into from

Conversation

paroga
Copy link
Member

@paroga paroga commented Mar 11, 2016

No description provided.

@wvengen
Copy link
Member

wvengen commented Mar 11, 2016

Awesome! I've been wanting to add a proper API. I would like to move to active model serializer at some point, this is a good start (Rails 5 has settled on that as well).

@wvengen
Copy link
Member

wvengen commented Mar 11, 2016

If you happened to have started on specifications for other endpoints, please feel free to add something to the wiki.

@wvengen
Copy link
Member

wvengen commented Mar 11, 2016

The only thing I'm missing here, are tests. Most important other things have tests now, and starting off the API without tests I'm afraid that won't get any coverage at all. For the rest, I'd be happy to merge it!

@paroga
Copy link
Member Author

paroga commented Mar 11, 2016

Will do so, but I want to upstream the basic "infrastructure" for it first (dorrkeeper).
My first usecase is to create a mobil-app for handling the "offline paper". I'd like to do some prototyping first (maybe in a plugin), get then some feedback and then upstream (and document) it.

+1 for adding tests. I only like to upstream the Api::V1::BaseController for now. Are there any good examples for similar tests?

@wvengen
Copy link
Member

wvengen commented Mar 11, 2016

Excerpt from another project (not using doorkeeper):

# spec/controllers/api/v1/user/status_controller_spec.rb
require 'rails_helper'

describe API::V1::User::StatusController do
  let(:default_params) { { api_version: "1.0" } }

  describe "GET to :index" do
    let(:user)  { create(:user) }
    let(:token) { user.authentication_token }

    before { get :index, authentication_token: token, format: "json" }

    it { is_expected.to respond_with :success }

    describe "the returned JSON" do
      subject(:json) { result = JSON.parse(response.body) }

      context "with a valid auth token" do

        its(["logged_in"]) { is_expected.to be true }

        describe "the included user" do
          subject { json["user"] }

          its(["id"]) { is_expected.to eq user.id }
        end
      end
    end
  end
end

and a serializer spec:

# spec/serializers/user_serializer_spec.rb
require 'rails_helper'

describe UserSerializer do
  let(:user) { create :user }
  let(:json) { described_class.new(user).to_json }
  subject(:parsed_json) { JSON.parse(json) }

  context "serializes a user without oauth_hash" do
    its(["email"]) { is_expected.to eq (user.email) }
    its(["can_preview"]) { is_expected.to be false }
  end
end

This uses rspec-its.

@wvengen
Copy link
Member

wvengen commented Mar 11, 2016

Regarding testing, perhaps using JSON schema may be even better.

@wvengen wvengen mentioned this pull request May 23, 2016
13 tasks
@wvengen
Copy link
Member

wvengen commented May 23, 2016

Closing in favour of #429. @paroga would the proposed financial_transactions endpoint and serializer be ok for you?

@wvengen wvengen closed this May 23, 2016
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.

2 participants