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

API v2: Planning Issue #3520

Open
stefannibrasil opened this Issue Sep 27, 2018 · 2 comments

Comments

Projects
2 participants
@stefannibrasil
Contributor

stefannibrasil commented Sep 27, 2018

@milaaraujo and I worked part-time on the API on our Rails Girls Summer of Code program. We were able to remove some duplicated code and refactor the whole Search feature.

But we couldn't finish it all and to leave a documentation for future work, there are some improvements that can be made:

  • Refactor the search.rb Class. It's too coupled with the DocResult class. Besides that, we notice that to create a DocResult, the endpoints have only small differences between them, so that could be abstracted to another service to reduce the duplicated code there.

  • Change all endpoints to use a search_criteria. We couldn't work on all the endpoints during our program, but the ones we worked on using a search_criteria valid object to access the params. The ideal would be to have all the endpoints to do the same (more details are given below).

  • Add more tests

  • Change version from v1 to v2

  • API next steps?

@jywarren

This comment has been minimized.

Show comment
Hide comment
@jywarren

jywarren Sep 27, 2018

Contributor
Contributor

jywarren commented Sep 27, 2018

@stefannibrasil

This comment has been minimized.

Show comment
Hide comment
@stefannibrasil

stefannibrasil Sep 29, 2018

Contributor

About the search_criteria and how to explore that part:

right now, the ExecuteSearch can receive a search_criteria object with all the params already validated:

https://github.com/publiclab/plots2/blob/master/app/services/execute_search.rb#L8

Right now only search_all and search_profiles endpoints receive a search_criteria object, so on SearchService we only call the params we want, instead of passing all the params. We could then change the other endpoints to use a search_criteria as well, so they would all follow a standard practice.

It's worth noticing that the SearchCriteria class is the place to add the default options, like the limit and the order_direction options. We can help with anyone is interested on doing this, it's a good way to learn how to refactor :)

Contributor

stefannibrasil commented Sep 29, 2018

About the search_criteria and how to explore that part:

right now, the ExecuteSearch can receive a search_criteria object with all the params already validated:

https://github.com/publiclab/plots2/blob/master/app/services/execute_search.rb#L8

Right now only search_all and search_profiles endpoints receive a search_criteria object, so on SearchService we only call the params we want, instead of passing all the params. We could then change the other endpoints to use a search_criteria as well, so they would all follow a standard practice.

It's worth noticing that the SearchCriteria class is the place to add the default options, like the limit and the order_direction options. We can help with anyone is interested on doing this, it's a good way to learn how to refactor :)

@stefannibrasil stefannibrasil added the Ruby label Sep 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment