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

Pulse sparkline is rendering dot in the wrong place #2037

Closed
ryanatflip opened this issue Feb 26, 2016 · 11 comments · Fixed by #2360
Closed

Pulse sparkline is rendering dot in the wrong place #2037

ryanatflip opened this issue Feb 26, 2016 · 11 comments · Fixed by #2360
Labels
Reporting/Pulses Now called Subscriptions Type:Bug Product defects
Milestone

Comments

@ryanatflip
Copy link

ryanatflip commented Feb 26, 2016

Please fill in the blanks, if relevant:

  • I am using the chrome._ browser.
  • My computer's OS is El Capitan_.
  • My database is _____.
  • My Metabase version is 0.13.2._.

Here is the query:
SELECT
date(created_at), count(1) AS "Total"
From
listings
WHERE
date(created_at) > current_date - interval '7 days' AND
reward = false and
PRICE IS NOT NULL AND
status = 'available'
GROUP BY 1
ORDER BY 1 Desc;

the results are attatched
screen shot 2016-02-26 at 1 25 06 pm

@salsakran salsakran changed the title Pulse bug Pulse sparkline is rendering dot in the wrong place Feb 26, 2016
@salsakran salsakran added the Type:Bug Product defects label Feb 26, 2016
@salsakran
Copy link
Contributor

Screenshot:
screenshot

@camsaul
Copy link
Member

camsaul commented Feb 26, 2016

Is this in an email, I'm assuming? There's been a lot of Pulse-related fixes in the four releases that have come out since 0.13.2. This might have been fixed since then

@camsaul camsaul added the Reporting/Pulses Now called Subscriptions label Feb 26, 2016
@ryanatflip
Copy link
Author

It is in an email, but the graph also looks like that within metabase.

How can I update our version of metabase?

On Fri, Feb 26, 2016 at 2:50 PM, Cam Saül notifications@github.com wrote:

Is this in an email, I'm assuming? There's been a lot of Pulse-related
fixes in the four releases that have come out since 0.13.2. This might
have been fixed since then


Reply to this email directly or view it on GitHub
#2037 (comment).

*Ryan Brennan *
Partnerships | Flip
http://t.sidekickopen40.com/e1t/c/5/f18dQhb0S7lC8dDMPbW2n0x6l2B9nMJW7t5XZs2zGvDCW3Mx_8F63BrBMW1qwhg656dMT9f3X6wT002?t=https%3A%2F%2Fflip.lease%2F&si=5222465635090432&pi=ba193c75-23df-4132-cd95-cbd28b144713

Press: The Atlantic
http://t.sidekickopen40.com/e1t/c/5/f18dQhb0S7lC8dDMPbW2n0x6l2B9nMJW7t5XZs2zGvDCW3Mx_8F63BrBMW1qwhg656dMT9f3X6wT002?t=http%3A%2F%2Fwww.citylab.com%2Fnavigator%2F2015%2F05%2Fhow-a-new-app-could-make-it-easier-to-break-your-lease%2F393119%2F&si=5222465635090432&pi=ba193c75-23df-4132-cd95-cbd28b144713
| Inman
http://t.sidekickopen40.com/e1t/c/5/f18dQhb0S7lC8dDMPbW2n0x6l2B9nMJW7t5XZs2zGvDCW3Mx_8F63BrBMW1qwhg656dMT9f3X6wT002?t=http%3A%2F%2Fwww.inman.com%2F2015%2F05%2F20%2Fflip-lets-renters-buy-and-sell-leases%2F&si=5222465635090432&pi=ba193c75-23df-4132-cd95-cbd28b144713
| Realtor.Com
http://www.realtor.com/advice/rent/flip-app-get-out-of-lease/

Find Us:: [image: social icons-08.png]
https://www.facebook.com/theflipapp
http://t.sidekickopen40.com/e1t/c/5/f18dQhb0S7lC8dDMPbW2n0x6l2B9nMJW7t5XZs2zGvDCW3Mx_8F63BrBMW1qwhg656dMT9f3X6wT002?t=https%3A%2F%2Fangel.co%2Fleaseflip%2F&si=5222465635090432&pi=ba193c75-23df-4132-cd95-cbd28b144713

@camsaul
Copy link
Member

camsaul commented Feb 26, 2016

Hi @ryanatflip, you can update Metabase the same way you installed it. See the installation guide for more details

@agilliland
Copy link
Contributor

if this is happening it would appear it's on SQL questions because i have several pulses that are rendering sparklines fine.

@ryanatflip
Copy link
Author

ryanatflip commented Feb 28, 2016 via email

@agilliland
Copy link
Contributor

I'm not sure I follow. from the screenshots shown above we'd expect to see 7 data points and they are all on the pulse sparkline. the problem as far as i understand it is that the dot on the sparkline is showing the first value instead of the last, as intended.

@tlrobinson
Copy link
Contributor

The pulse rendering code is totally separate from the chart rendering code since we haven't been able to embed a good enough browser engine in the Java backend yet to render our JavaScript charts, so it's totally plausible there are bugs that affect one but not the other.

If I were to guess, without trying your query or looking at the pulse rendering code, I'd say your query is returning the data in reverse chronological order, and we're not sorting it before picking the "last" data points for the labels and dot.

If that's the case you should be able to work around this bug by switching your query's ORDER BY clause from DESC to ASC.

@agilliland
Copy link
Contributor

@salsakran @kdoh @mazameli , can one of you weigh in on the desired UX here?

in this particular case the data points being highlighted are not the most recent points due to the fact that the data is sorted in the opposite order from what's expected. do we still want to highlight the most recent 2 data points? or is it okay that given the different sort order of the data that the oldest 2 data points are highlighted in this case?

@salsakran
Copy link
Contributor

The intent of the pulse sparkline is to give the user the most recent value and show them the trend.

The assumption "we expect the timeseries in ascending order" was (and is) reasonable. It would be nice if we would do the right thing even if this assumption is broken by the user.

@mazameli
Copy link
Contributor

Yeah, if the user does something weird that would make the chart look reversed, it would be nice if we just flipped it back so that the most recent data point is on the right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reporting/Pulses Now called Subscriptions Type:Bug Product defects
Projects
None yet
6 participants