joplin / plugin-kanban

It allows notes in a notebook to be organized in a kanban board.
https://discourse.joplinapp.org/t/kanban-board-project/17469
132 stars 17 forks source link

basic implementation for exclude filter #41

Closed mhelleboid closed 1 year ago

mhelleboid commented 1 year ago

Exclude filter

Implementation of exclude filter, which allows me to easily use "work in progress" column for todo with the following configuration :

columns:
  - name: Backlog
    completed: false
    tag: -wip
  - name: Work in progress
    completed: false
    tag: wip
  - name: Finished
    completed: true
    tag: -wip

May be a solution to #29

All Tags

Also added an "alltags" rule to ensure that all tags are present or not present, which allows for example to add a "Priority" column with the following configuration

columns:
  - name: Backlog
    completed: false
    alltags:
      - -prio
      - -wip
  - name: Priorities
    completed: false
    alltags:
      - prio
      - -wip
  - name: Work in progress
    completed: false
    alltags:
      - -prio
      - wip
  - name: Done
    completed: true
    alltags:
      - -prio
      - -wip

New Behavior

Change the behaviour of "completed: false" and "completed: true" statement when used with tag/tags statements. And may have other impacts, like for example the facts that column tag/tags and filters tag/tags must all be satisfied. See the test modification for the behaviour change

Next

I can try to write more tests if needed

PackElend commented 1 year ago

@mhelleboid we are still searching for a new maintainer but as you familiar (a bit) with the codebase of the plug-in would are ok with checking the other PRs. I can't call myself experienced enough, but if some of the others would check each other's PRs, it can be said that the code is safe, so I merge it. At least there would be some progress thx

mhelleboid commented 1 year ago

@PackElend, yes sure, I can have a look at opened PRs.

benlau commented 1 year ago

As suggested by @PackElend , I am here to help review the code.

I have confirmed that the code works as described, and the test case has passed.

When moving a note from a column with the "tag: -wip" label, it will set the "wip" flag. I am a bit confused at the first glance, but it should be a reasonable behavior.

The biggest impact is the change in the column filtering rule, which has been changed from "some" to "all." I think it makes sense, but it may cause a weird issue for the user.

To replicate the issue:

  1. Create a column with the rule of "completed: true" (or false)
  2. Attempt to drag a normal note (not a todo) to that column. It will not be possible

Furthermore, I found a minor defect (which will probably be ignored by the user and no need to fix?):

  1. In a kanban with multiple "-tag" tags column.
  2. Pressing "add" in the first column will add the new note to the last column with the label "-some_other_tag" instead of adding it to the intended column.
mhelleboid commented 1 year ago

Thanks @benlau for the review, I will take your feedback in consideration and update this PR!

PackElend commented 1 year ago

@mhelleboid how it is going with the update ?

mhelleboid commented 1 year ago

@PackElend I will try to work on it next weekend

mhelleboid commented 1 year ago

When moving a note from a column with the "tag: -wip" label, it will set the "wip" flag. I am a bit confused at the first glance, but it should be a reasonable behavior.

Yes, I think this is an acceptable behavior, removing this behavior leads to others less acceptable behabior

mhelleboid commented 1 year ago

The biggest impact is the change in the column filtering rule, which has been changed from "some" to "all." I think it makes sense, but it may cause a weird issue for the user.

Maybe this should be configurable, but I like the new behavior where the filtering rule actually filters notes @benlau what do you think?

mhelleboid commented 1 year ago

Furthermore, I found a minor defect (which will probably be ignored by the user and no need to fix?): In a kanban with multiple "-tag" tags column.

I cannot reproduce it, @benlau can you give me a more detailed scenario please?

benlau commented 1 year ago

The biggest impact is the change in the column filtering rule, which has been changed from "some" to "all." I think it makes sense, but it may cause a weird issue for the user.

Maybe this should be configurable, but I like the new behavior where the filtering rule actually filters notes @benlau what do you think?

@mhelleboid I think it is ok. We may not need to fix it.

I cannot reproduce it, @benlau can you give me a more detailed scenario please?

sorry, a bit busy for this week, what if we just ignore this issue as it is a very minor issue.

PackElend commented 1 year ago

@mhelleboid @benlau it looks like your peer review works quite well. Are you interested to become maintainers?

@mhelleboid do I find you n the forum?

mhelleboid commented 1 year ago

@mhelleboid @benlau it looks like your peer review works quite well. Are you interested to become maintainers?

I can try

@mhelleboid do I find you n the forum?

Just created an account with the same username