Add list completed #82
Conversation
|
nice, thanks for this! I'll have time to give it a review later today. |
|
@stuartskelton code looks great. I love the I test drove the code locally, and I think we should include archived todos in the completed list. Once that's done I think we can 🚢 ! |
|
let me see how I would do this. |
|
Ok I think that is it. Im not a 100% happy about how I had to achieve it but I am feeling a refactoring coming on. |
|
ah, yeah. I see what you had to do there. Archived todos were designed to be never shown when listing, which is why it's architected the way it is. I think this solution is the right one though. |
|
tested locally, this works perfectly. Thanks @stuartskelton ! |
todolist/date_filter_test.go
Outdated
|
|
||
| assert.Equal(0, len(filtered)) | ||
|
|
||
| // now to complted one and see see what happens |
There was a problem hiding this comment.
not sure if this comment is necessary, and it also has a typo.
There was a problem hiding this comment.
yeah lets remove it 😃
todolist/date_filter_test.go
Outdated
| assert.Equal(1, len(filtered)) | ||
| assert.Equal(1, filtered[0].Id) | ||
|
|
||
| // now to complted one and see see what happens |
|
@stuartskelton 🥇 looks perfect. Thanks for implementing this! I'll do a release this week. |
This is a working implementation of #67.
I have built upon my earlier work to reduce the redundancy in the date_filter.go file, passing a function that supplies the date to filter on, either the Due date or the CompletedDate (minus the timestamp provided by CompletedDateToDate).
I have renamed the filterToday and filterTomorrow for due filtering to filterDueToday and filterDueTomorrow respectively, in an effort to pass the focus of the filtering down to many levels. (I am happy to amend this view point). The reason I have not done this for 'filterThisWeek' is because I have another PR (#79) that will tidy this up a little.
The todolist.site might also need a little update if this is accepted too.