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

3222 spread custom fields api optimization #3283

Merged
merged 8 commits into from
Aug 31, 2020

Conversation

entantoencuanto
Copy link
Member

@entantoencuanto entantoencuanto commented Aug 4, 2020

Closes #3222

✌️ What does this PR do?

  • Extends custom fields API optimization to data and investments Gobierto modules
  • Adapts front components to new API response (for vocabulary custom fields the old response was always an array of serialized vocabulary terms and now is the term id)
  • Fixes an error mixing API optimization and filters

🔍 How should this be manually tested?

Visit affected modules. Nothing should change

:shipit: Does this PR changes any configuration file?

No

(Changes in these files might need to update the role in Ansible)

📖 Does this PR require updating the documentation?

No

@codecov
Copy link

codecov bot commented Aug 5, 2020

Codecov Report

Merging #3283 into master will increase coverage by 0.00%.
The diff coverage is 83.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3283   +/-   ##
=======================================
  Coverage   84.76%   84.77%           
=======================================
  Files         804      804           
  Lines       21275    21277    +2     
=======================================
+ Hits        18034    18037    +3     
+ Misses       3241     3240    -1     
Impacted Files Coverage Δ
...ollers/gobierto_data/api/v1/datasets_controller.rb 90.00% <75.00%> (+0.20%) ⬆️
...gobierto_investments/api/v1/projects_controller.rb 95.65% <100.00%> (ø)
...ierto_budgets/budgets_execution/metric_box_cell.rb 100.00% <0.00%> (+3.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f81191...29a7f5a. Read the comment docs.

@entantoencuanto
Copy link
Member Author

Please, @Crashillo, could you review the js changes before I merge into staging?

@@ -374,8 +374,9 @@ export default {
// Get the items based on these new active filters
const __items__ = this.applyFiltersCallbacks(__activeFilters__);

return __items__.filter(({ attributes }) =>
attributes[key].map(g => g.id).includes(id)
return __items__.filter(({ attributes }) =>(
Copy link
Member

Choose a reason for hiding this comment

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

You may get rid of the wrapping parenthesis, in the filter condition

@entantoencuanto entantoencuanto marked this pull request as ready for review August 6, 2020 12:09
@ferblape ferblape merged commit 0ba1360 into master Aug 31, 2020
@ferblape ferblape deleted the 3222-spread_custom_fields_api_optimization branch August 31, 2020 05:04
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

Successfully merging this pull request may close these issues.

Spread custom fields api optimizations to the rest of APIs
3 participants