Skip to content
GitHub no longer supports this web browser. Learn more about the browsers we support.
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

Recent update ordering: Current week always before previous week #287

Closed
mtlynch opened this issue Sep 16, 2019 · 1 comment
Closed

Recent update ordering: Current week always before previous week #287

mtlynch opened this issue Sep 16, 2019 · 1 comment

Comments

@mtlynch
Copy link
Owner

@mtlynch mtlynch commented Sep 16, 2019

There's a bug in the ordering in the response of recentEntriesGet (alluded to in dtq's recent update) where you can bump a previous week's update to the front of the recent entries list by making an edit to it.

This isn't the ideal behavior.

Desired behavior

Order updates like:

Order first by week of update, then by last modified time (in descending order):

  1. alice's update for the week of 2019-09-20 (last modified @ 2019-09-20T18:00:00Z)
  2. bob's update for the week of 2019-09-20 (last modified @ 2019-09-20T12:00:00Z)
  3. charlie's update for the week of 2019-09-20 (last modified @ 2019-09-20T10:00:00Z)
  4. alice's update for the week of 2019-09-13 (last modified @ 2019-09-20T19:00:00Z)

Current behavior

Order exclusively by last modified time (in descending order), which sometimes bumps previous weeks ahead of the current week:

  1. alice's update for the week of 2019-09-13 (last modified @ 2019-09-20T19:00:00Z) << wrong
  2. alice's update for the week of 2019-09-20 (last modified @ 2019-09-20T18:00:00Z)
  3. bob's update for the week of 2019-09-20 (last modified @ 2019-09-20T12:00:00Z)
  4. charlie's update for the week of 2019-09-20 (last modified @ 2019-09-20T10:00:00Z)
@mtlynch

This comment has been minimized.

Copy link
Owner Author

@mtlynch mtlynch commented Sep 20, 2019

I've added a unit test that demonstrates correct functionality:

https://github.com/mtlynch/whatgotdone/compare/recent-order?expand=1

This unit test currently fails.

Anyone interested in contributing to What Got Done can fix this issue by making the unit test pass (you don't have to register an account anywhere or set up databases):

To fix this bug:

  1. Fork this repo
  2. Clone your fork locally
  3. To pull in the unit test and run it:
git remote add mtlynch https://github.com/mtlynch/whatgotdone.git
git checkout -b recent-order
git pull mtlynch recent-order
go test ./...

The test will fail because of the bug. Make necessary changes to backend/handers/recent_entries.go to make the test pass.

Push to your fork and make a pull request when the unit test passes.

enc pushed a commit to enc/whatgotdone that referenced this issue Sep 25, 2019
enc pushed a commit to enc/whatgotdone that referenced this issue Sep 26, 2019
fixes mtlynch#287

Better order for journal entries in test
@mtlynch mtlynch closed this in 2977709 Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.