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

Complaints and Notes from integrating API v3 into Flappi. #26

Open
kylorhall opened this issue Jul 11, 2018 · 0 comments
Open

Complaints and Notes from integrating API v3 into Flappi. #26

kylorhall opened this issue Jul 11, 2018 · 0 comments

Comments

@kylorhall
Copy link
Contributor

kylorhall commented Jul 11, 2018

@richdrich Just dropping these in here for now for visibility.

This can, and will, be split out into other issues, just a place for my notes right now. I will update it as I go.


[easy] alias objects => collection would make things more scannable


Developing against it is very difficult for someone new. I don't know if anyone could take this project over. The @delegate magic and the loading based on things that aren't visible to Flappi make it quite difficult. Flappi assumes Investapp's folder structure rather than Investapp telling Flappi what to do. You cannot work on Flappi in encapsulation – you must have knowledge of the parent in order to understand the child.

Or maybe it's all just typical Rails assumptions that I'm avoidant to and there's nothing wrong, I don't know.


I can't seem to find any way to make reusable (but different) definitions with Flappi due to DocumentingStub. If I do something like object.key?(:value), it stubs it to true. If I do include?, it seems to stub it. Everything seems to get stubbed. Actually, straight up syntax get stubbed. Too much stubbing. But it doesn't stub global variables (@portfolio)? (and I'm perfectly fine with that)

The magic is too strong for me..


Links are top-level only, I can't have nested links without doing it manually via object :links / field :link. Take api/v2/portfolios – ideally we would have a top-level links and a nested link or links – either is fine – each Portfolio should have a link to that Portfolio. I can't quite see the use case for links as they exist today, actually.


Resolved:

links seems to just be broken sometimes. Take holdings_show.rb => path='/holdings/:id'. I might be missing something – just confused why it always prepends the path sometimes, but not always.

link :portfolio, "portfolios/#{holding.portfolio.id}" # becomes `api/v3/holdings/:id/portfolios/1234`
link :self # becomes `api/v3/holdings/:id/holdings/:id`

If I want to throw an error, I can't do it in the controller, easily. If I want to validate something, I have to do it in query, or manually return the error outside of Flappi. I can't find a case now, but things like loading Portfolio, Holdings, etc, require this sort of boilerplate in each query vs. a Controller method:

query do
  portfolio = master_current_user.find_portfolio(params[:portfolio_id], { consolidated: params[:consolidated] })
  raise Api::MessageError.new(BaseController::NO_PORTFOLIO_FOUND_MESSAGE, BaseController::NO_ENTRY_FOUND) unless portfolio
  
  # …
end

And my main dislike is this:

Almost all of the logic goes into the view file (definition) – the view file is also the controller and logic. However, if you need to make something like path 'holdings/:holding_id' named :holding_id so documentation looks nicer, you have to do it in the first controller.

Effectively I get:
routes.rb
=> api/v3/controller.rb which acts as a router, with some hacks required to make params work magically in Flappi
=> api/v3/api_builder_definitions/method.rb which acts as a controller and a view template

You can't really use the controllers – things like before_action :load_portfolio and logic like before_action :verify… will never really work as @portfolio works fine in "response mode", but throws errors in "definition mode".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant