fix: rely on api to properly sort tasks on home page #1997

konrad · 2022-05-28T09:28:09Z

konrad commented

2022-05-28 09:28:09 +00:00

This PR changes the behaviour of how tasks are sorted. Before, the frontend would sort tasks but this resulted in some cases where tasks were not sorted properly. Most of this is test code to reliably reproduce the problem and make fixing it easier.
The actual bug was in Vikunja's api, therefore I've removed all sorting of tasks in the frontend and ensured the api properly sorts tasks.

Fixes https://github.com/go-vikunja/frontend/issues/54

Depends on vikunja/api#1177

This PR changes the behaviour of how tasks are sorted. Before, the frontend would sort tasks but this resulted in some cases where tasks were not sorted properly. Most of this is test code to reliably reproduce the problem and make fixing it easier. The actual bug was in Vikunja's api, therefore I've removed all sorting of tasks in the frontend and ensured the api properly sorts tasks. Fixes https://github.com/go-vikunja/frontend/issues/54 Depends on https://kolaente.dev/vikunja/api/pulls/1177

konrad added 4 commits 2022-05-28 09:28:09 +00:00

9d6822a75f

fix: add tests to make sure tasks are sorted correctly on overview

de440ce097

fix: let the api sort through tasks properly

68c9d2d9a1

feat: add more test cases for overdue and tasks via default list

continuous-integration/drone/pr Build is passing Details

638bac9e10

fix: rely on the api sorting tasks properly and don't try to override the order in the frontend

konrad commented

2022-05-28 09:28:52 +00:00

Tests will fail since the api used in cypress tests is the old one without the fix. Will restart once the api PR is merged.

frederick commented

2022-05-28 09:39:05 +00:00

Hi konrad!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://1997-fix-due-soon-first--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi konrad! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://1997-fix-due-soon-first--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.

dpschen reviewed 2022-05-30 10:41:28 +00:00

cypress/integration/task/overview.spec.js Outdated

						
				@ -0,0 +34,4 @@

					return {tasks, list}

				}

				import '../../support/authenticateUser'

dpschen commented

2022-05-30 10:38:48 +00:00

Move imports to top

konrad commented

2022-05-31 19:29:28 +00:00

Done.

konrad marked this conversation as resolved

cypress/integration/task/overview.spec.js

						
				@ -0,0 +106,4 @@

						}, false)

						cy.visit('/')

						cy.get('[data-cy="showTasks"] .card .task')

dpschen commented

2022-05-30 10:39:34 +00:00

Don't you remove the task class?

konrad commented

2022-05-30 20:54:32 +00:00

The <single-task-in-list/> component still has it.

The `<single-task-in-list/>` component still has it.

👍 1

dpschen commented

2022-06-01 05:08:52 +00:00

I was mostly asking because I remember the task class giving styling to some childs. But if that doesnt't apply here, all good :)

konrad changed title from ~~WIP: fix: rely on api to properly sort tasks on home page~~ to fix: rely on api to properly sort tasks on home page

2022-05-31 19:27:38 +00:00

konrad added 1 commit 2022-05-31 19:29:25 +00:00