This repository has been archived by the owner on Jan 12, 2021. It is now read-only.
Fix next_page on Dashboard.recent and Stat.latest_for_teams #41
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevintyll
suggested changes
Oct 18, 2016
| @@ -82,7 +93,7 @@ def self.for_report(report_id = nil, options = {}) # rubocop:disable Style/Optio | |||
| # @return [ActiveResource::PaginatedCollection<ESP::Stat>] | |||
| def self.latest_for_teams | |||
| # call find_every directly since find is overriden/not implemented | |||
| find_every(from: :latest_for_teams) | |||
| find_every(from: "#{prefix}stats/latest_for_teams") | |||
There was a problem hiding this comment.
I think if you change this to where, you won't have to override find_every
There was a problem hiding this comment.
Yeah that seems to work. I should have started with the where override that allowed it.
¯_(ツ)_/¯
| @@ -38,7 +49,7 @@ def destroy | |||
| # @return [ESP::Dashboard] | |||
| def self.recent | |||
| # call find_every directly since find is overridden/not implemented | |||
| find_every from: "#{prefix}dashboard/recent.#{format.extension}" | |||
| find_every from: "#{prefix}dashboard/recent" | |||
There was a problem hiding this comment.
I think if you change this to where, you won't have to override find_every
kevintyll
approved these changes
Oct 19, 2016
|
👍 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Calling
next_pageonDashboard.recentorStat.latest_for_teamswould return a merge for nil error. This turned out to be because the pagination wasn't set up when callingfind_every. After fixing that it was discovered that thePaginatedCollectionuseswherefornext_pagecalls so that had to be enabled.This also requires some server side changes due to
wheremaking a PUT request. PUT was not supported for these two endpoints previously.I also discovered that
next_pagewould cause a server side error on other APIs. This was because the logic inwherethat parses the page numbers was not always a symbol key, causing the page parameter to get merged into the filter params. I fixed this by adding awith_indifferent_accesson the params before they are parsed.This fixes the reopening of #33.