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

Switch to query2 #48

Merged
merged 30 commits into from Jan 22, 2018

Conversation

Projects
None yet
2 participants
@johan-bjareholt
Copy link
Member

johan-bjareholt commented Oct 7, 2017

Replaced all javascript transformation with query2 requests and removed the last View stuff

  • Fix timeline being inconsistent when "Show AFK time" is toggled
  • Fix timeline being broken when "Show AFK time" isn't toggled
  • Non-hardcoded web bucket

@wafflebot wafflebot bot added review and removed in progress labels Oct 14, 2017

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented Oct 16, 2017

We got this new issue today: ActivityWatch/activitywatch#117

So I'm wondering if you've tested that data is fetched using the right timezone? (you can check this by seeing if data is properly queried around midnight)

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented Oct 16, 2017

I think your code is likely to suffer from the same issue, I think I fixed it in this PR so you shouldn't have to worry about it: #49

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented Oct 16, 2017

By the way, how is "Show AFK time" broken/inconsistent?

It had issues before you started working on query2. I'm fine with just adding a notification that it's partially broken for now if it's something you don't feel like putting a lot of work into (assuming a fix is non-trivial).

@johan-bjareholt

This comment has been minimized.

Copy link
Member

johan-bjareholt commented Oct 17, 2017

The start- and endtime should be the same as previous queries so it shouldn't differ, so i should probably rebase to your commit later.

The "Show AFK time" is inconsistent due to the timeline used the "total_duration" field in chunks, and since we now use the query2 merge function instead which doesn't have the total_duration field i did a second "total_duration" query. Generally that requests returns before the timeline query, but not always and in that case it can completely break so you have to refresh to fix it. Could be a trivial fix, but we'll see.

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented Oct 17, 2017

Can't you do the total_duration calculation client-side? Should be easy enough... Seems a bit overkill to make a query for it.

Replaced totalTimeQuery with clientside calculation
Since the data is already avaliable it's kind of excessive to use a
query for such a simple thing.

@johan-bjareholt johan-bjareholt force-pushed the dev/transform-next branch from 5e55cbe to 99832e4 Oct 21, 2017

@johan-bjareholt

This comment has been minimized.

Copy link
Member

johan-bjareholt commented Oct 21, 2017

Lol, github doesn't like rebases for sure, commit history looks insane

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented on 99832e4 Oct 23, 2017

Nice 👍

RETURN=events';
windowTimelineQuery: function(windowbucket, afkbucket){
return { "query": [
'not_afk = query_bucket("'+afkbucket+'");',

This comment has been minimized.

@ErikBjare

ErikBjare Nov 19, 2017

Member

All variables used in the query need to be sanitized, we should at the very least add a TODO/FIXME comment.

This comment has been minimized.

@johan-bjareholt
@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented on 6410170 Nov 26, 2017

Really nice that you refactored the Sunburst stuff. I thought of doing that myself but was pressed for time.

This comment has been minimized.

Copy link
Member

johan-bjareholt replied Nov 26, 2017

Well it's not really refactored, I only moved it. The Activity.vue file was starting to get annoyingly long which was the reason. There's a bit of refactoring needed around in the aw-webui code though.

@ErikBjare

This comment has been minimized.

Copy link
Member

ErikBjare commented Jan 22, 2018

Merging this!

@ErikBjare ErikBjare merged commit fca040e into master Jan 22, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@wafflebot wafflebot bot removed the review label Jan 22, 2018

@ErikBjare ErikBjare deleted the dev/transform-next branch Jan 22, 2018

@ErikBjare ErikBjare changed the title WIP: transform-next Switch to query2 May 16, 2018

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