vikunja
/
frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: mark undone if task moved from isDoneBucket #3291

Merged
konrad merged 1 commits from WofWca/frontend:mark-undone-after-moving-from-done-bucket into main 2023-03-28 10:21:23 +00:00
Conversation 9 Commits 1 Files Changed 1 +8
1 changed files with 8 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/views/project/ProjectKanban.vue
View File

@ -415,6 +415,7 @@ async function updateTaskPosition(e) {
: e.newIndex
const task = newBucket.tasks[newTaskIndex]
const oldBucket = buckets.value.find(b => b.id === task.bucketId)
const taskBefore = newBucket.tasks[newTaskIndex - 1] ?? null
const taskAfter = newBucket.tasks[newTaskIndex + 1] ?? null
taskUpdating.value[task.id] = true
@ -425,6 +426,13 @@ async function updateTaskPosition(e) {
taskBefore !== null ? taskBefore.kanbanPosition : null,
taskAfter !== null ? taskAfter.kanbanPosition : null,
)
if (
oldBucket != undefined && // This shouldn't actually be `undefined`, but let's play it safe.
dpschen commented 2023-03-23 16:58:23 +00:00
Outdated
Review
Copy Link

If I'm not wrong the bucketId could be 0. If that's the case we really should check for undefined instead.

If I'm not wrong the `bucketId` could be `0`. If that's the case we really should check for `undefined` instead.
WofWca commented 2023-03-23 17:52:09 +00:00
Outdated
Review
Copy Link

Unless we can specify the reason for it being 0 I'm for keeping the comment. Makes it clear that we don't actually know it.

Unless we can specify the reason for it being `0` I'm for keeping the comment. Makes it clear that we don't actually know it.
dpschen commented 2023-03-23 18:19:08 +00:00
Outdated
Review
Copy Link

Its the id of the default bucket. The comment is fine

Its [the id of the default bucket](https://kolaente.dev/vikunja/frontend/src/commit/02971f6ff9c4dd98014874dbb544f4fad66eb0ab/src/models/bucket.ts#L10). The comment is fine
WofWca commented 2023-03-23 18:22:08 +00:00
Outdated
Review
Copy Link

By "default bucket" you mean a newly created Bucket instance, right? I don't think it's currently possible to move tasks from a bucket that hasn't been sent to the backend yet, is it?

By "default bucket" you mean a newly created `Bucket` instance, right? I don't think it's currently possible to move tasks from a bucket that hasn't been sent to the backend yet, is it?
konrad commented 2023-03-23 20:31:58 +00:00
Outdated
Review
Copy Link

The default bucket is the leftmost bucket, where tasks are created (or moved to) when no bucket is specified.

The default bucket is the leftmost bucket, where tasks are created (or moved to) when no bucket is specified.
WofWca commented 2023-03-24 07:06:23 +00:00
Outdated
Review
Copy Link

The default bucket is the leftmost bucket

IMU code is run when you drag a task between buckets or within a bucket, so the said task should always have bucketId set.

> The default bucket is the leftmost bucket IMU code is run when you drag a task between buckets or within a bucket, so the said task should always have `bucketId` set.
konrad commented 2023-03-24 11:04:49 +00:00
Outdated
Review
Copy Link

Yeah in the frontend there should always be a bucket id set. What is was describing is the api behaviour.

Yeah in the frontend there should always be a bucket id set. What is was describing is the api behaviour.
dpschen commented 2023-03-27 21:13:20 +00:00
Outdated
Review
Copy Link

By 'set' you mean 'some other value than 0?

My original intend here mostly was: I see it as bad style ff a possible value of a variable is 0 (which is true here, even if it might get replaced with something different shortly after). Because of that I thought:

oldBucket !== undefined && // This shouldn't be `undefined`, but let's play it safe.

Might make sense here.

By 'set' you mean 'some other value than `0`? My original intend here mostly was: I see it as bad style ff a possible value of a variable is `0` (which is true here, even if it might get replaced with something different shortly after). Because of that I thought: ```ts oldBucket !== undefined && // This shouldn't be `undefined`, but let's play it safe. ``` Might make sense here.
konrad commented 2023-03-28 09:56:00 +00:00
Outdated
Review
Copy Link

By 'set' you mean 'some other value than 0?

yes, exactly.

> By 'set' you mean 'some other value than 0? yes, exactly.
WofWca commented 2023-03-28 10:02:10 +00:00
Outdated
Review
Copy Link

By 'set' you mean 'some other value than 0?

Yes, since the back-end can't have an actual bucket with an id of 0.

I see it as bad style ff a possible value of a variable is 0

Wait, are you saying that the value of oldBucket could be 0? No, it's either a bucket object, or undefined (the latter shouldn't happen). But ok, I'll update it to != undefined to cover null and undefined, but not 0.

> By 'set' you mean 'some other value than 0? Yes, since the back-end can't have an actual bucket with an `id` of 0. > I see it as bad style ff a possible value of a variable is 0 Wait, are you saying that the value of `oldBucket` could be `0`? No, it's either a bucket object, or undefined (the latter shouldn't happen). But ok, I'll update it to `!= undefined` to cover `null` and `undefined`, but not 0.
konrad commented 2023-03-28 10:02:56 +00:00
Outdated
Review
Copy Link

Even if you set it to 0 the api will set it to the default bucket.

Even if you set it to `0` the api will set it to the default bucket.
dpschen commented 2023-03-28 10:06:00 +00:00
Outdated
Review
Copy Link

Wait, are you saying that the value of oldBucket could be 0?

The initial set value in the frontend of bucketId is 0.

So even if that might be replaced in the moment the API answers it currently is true for short period of time. If this is not wished, we should adjust the initial value in the model (e.g. as you said @WofWca to undefined).

Note I talk here about the bucketId and not the oldBucket. But since the latter is based on the former I thought not relying on implicit type conversion would be clearer.

> Wait, are you saying that the value of oldBucket could be 0? The initial set value in the frontend of `bucketId` is `0`. So even if that might be replaced in the moment the API answers it currently is true for short period of time. If this is not wished, we should adjust the initial value in the model (e.g. as you said @WofWca to `undefined`). Note I talk here about the `bucketId` and not the `oldBucket`. But since the latter is based on the former I thought not relying on implicit type conversion would be clearer.
WofWca commented 2023-03-28 10:08:40 +00:00
Outdated
Review
Copy Link

The oldBucket variable is the return value of the find function that is called on the array of bucket objects. Are you taking about oldBucket or task.bucketId?

Edit: saw the edit.

The `oldBucket` variable is the return value of the `find` function that is called on the array of bucket objects. Are you taking about `oldBucket` or `task.bucketId`? Edit: saw the edit.
👍 1
dpschen commented 2023-03-28 10:12:27 +00:00
Outdated
Review
Copy Link

Either way, I don't care anymore :D This is way too much effort for way to few impact. Change as you wish ;)

Either way, I don't care anymore :D This is way too much effort for way to few impact. Change as you wish ;)
😆 1
newBucket.id !== oldBucket.id &&
newBucket.isDoneBucket !== oldBucket.isDoneBucket
) {
newTask.done = newBucket.isDoneBucket
}
try {
await taskStore.update(newTask)