Skip to content
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

Load task from server if not found locally #608

Merged
merged 3 commits into from Sep 20, 2019
Merged

Conversation

raimund-schluessler
Copy link
Member

@raimund-schluessler raimund-schluessler commented Sep 13, 2019

This implements #606. When a requested task is not found locally, we try to load it from the server.

Todo:

  • We should also try to load the ancestor tasks (if any) of the requested task, so we can properly show the subtasks tree
  • If we are in a collection view (e.g. /nextcloud/index.php/apps/tasks/#/collections/all/tasks/Nextcloud-lrr0rvhdcoqopte5k43n6.ics), we don't know which calendar a task belongs to, so we have to query all calendars I guess. Or is there a better way to find a task only knowing the last part of its uri @georgehrke @skjnldsv ?
  • Don't loose subtask relations for tasks alread present in store when loading tasks by Uri or UID

What I don't like so much is that I have to watch the calendar (https://github.com/nextcloud/tasks/pull/608/files#diff-128fe9ee0ed5aeabe3c64281c67285c0R584), but it was the easiest way to load a task only after the calendars are loaded.

@codecov
Copy link

codecov bot commented Sep 13, 2019

Codecov Report

Merging #608 into master will decrease coverage by 0.16%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master    #608      +/-   ##
=========================================
- Coverage    5.82%   5.66%   -0.17%     
=========================================
  Files          45      45              
  Lines        1939    1996      +57     
  Branches      343     351       +8     
=========================================
  Hits          113     113              
- Misses       1679    1730      +51     
- Partials      147     153       +6

@raimund-schluessler
Copy link
Member Author

This should be good to go now.

src/store/tasks.js Outdated Show resolved Hide resolved
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments :)

@raimund-schluessler
Copy link
Member Author

See comments :)

Fixed it. Looks much better now with async/await.

src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
src/store/tasks.js Outdated Show resolved Hide resolved
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>

Co-Authored-By: John Molakvoæ <skjnldsv@users.noreply.github.com>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Copy link
Member Author

Rebased.

@raimund-schluessler raimund-schluessler merged commit 5dee2b0 into master Sep 20, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix-606 branch September 20, 2019 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants