fix: partial fix to allow list tasks in ios reminders app #2717

Merged
konrad merged 3 commits from jd/vikunja:main into main 2024-09-28 09:06:34 +00:00
Contributor

This PR introduces a partial fix for the CalDAV task listing bug (#753) when handling PROPFIND requests with Depth: 1, improving task visibility in the iOS Reminders app.

Notes:

  • This might make Thunderbird somewhat usable when interacting with tasks using the /dav/projects/{id} url.
  • This does not fully resolve the issue where the Reminders app will only display the last project after some time when adding the URL.

This is my first time working with Golang and CalDAV, so I’d really appreciate any feedback or suggestions on the code structure, style, or any improvements I could make.

This PR introduces a partial fix for the CalDAV task listing bug (#753) when handling PROPFIND requests with `Depth: 1`, improving task visibility in the iOS Reminders app. Notes: * This might make Thunderbird somewhat usable when interacting with tasks using the `/dav/projects/{id} url`. * This does not fully resolve the issue where the Reminders app will only display the last project after some time when adding the URL. This is my first time working with Golang and CalDAV, so I’d really appreciate any feedback or suggestions on the code structure, style, or any improvements I could make.
jd added 2 commits 2024-09-24 21:36:23 +00:00
- This addresses the bug where tasks were not being listed properly in certain cases.
fix: check if vcls.project.Tasks != nil before listing task
Some checks failed
continuous-integration/drone/pr Build is failing
4b05986186
konrad requested changes 2024-09-25 15:21:09 +00:00
Dismissed
konrad left a comment
Owner

Looks pretty good already!

Please fix the lint issues. Can you also add a test for this?

Looks pretty good already! Please fix the lint issues. Can you also add a test for this?
@ -95,0 +96,4 @@
// If the request is withChildren (Depth: 1), we need to return all tasks of the project
if withChildren {
var resources []data.Resource
resources = append(resources, r)
Owner

You could initialize this as resource = []data.Resource{r}

You could initialize this as `resource = []data.Resource{r}`
@ -95,0 +108,4 @@
}
taskResourceInstance := data.NewResource(getTaskURL(&vcls.project.Tasks[i].Task), &taskResource)
taskResourceInstance.Name = vcls.project.Tasks[i].Title
resources = append(resources, taskResourceInstance)
Owner

Can you extract this out in a new function? A very similar logic is used in GetResourcesByList below.

Can you extract this out in a new function? A very similar logic is used in `GetResourcesByList` below.
jd added 1 commit 2024-09-28 01:27:12 +00:00
chore: refactor task resource creation
All checks were successful
continuous-integration/drone/pr Build is passing
36102f7b57
Member

Hi jd!

Thank you for creating a PR!

I've deployed the frontend changes of this PR on a preview environment under this URL: https://2717-main--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 somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi jd! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2717-main--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 somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.
konrad approved these changes 2024-09-28 09:06:01 +00:00
konrad left a comment
Owner

Thanks!

Thanks!
konrad merged commit 84dbc5fd84 into main 2024-09-28 09:06:34 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/vikunja#2717
No description provided.