10000 Add Feature: Board templates or other stuff by floatinghotpot · Pull Request #446 · wekan/wekan · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Feature: Board templates or other stuff #446

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

Closed
wants to merge 3 commits into from

Conversation

floatinghotpot
Copy link
Contributor

clone board template from another board to current board, including members, labels, and lists, thus a new board can be quickly setup.

Review on Reviewable

@mquandalle
Copy link

Generally speaking I would remove the notion of a “board template” from the code base in favor of simpler “clone” feature, I exposed this idea in #308 (comment).

I’m also experimenting (for the first time with this PR) with a new code review application. It has happen a few times that I put a comment on a GitHub PR, and then the code change so GitHub hide the comment whereas it’s still not addressed. So we will see if Reviewable.io improve things or not, feel free to comment if you have an opinion on this :-)


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 6 unresolved discussions.


client/components/boards/boardHeader.jade, line 85 [r1] (raw file):
Not saying this is fundamentally better but you can avoid the creation of a custom helper here by using `($.Session.equals 'currentBoard' _id) which is provided by raix:handlebar-helpers.

Anyway this whole class of issues will disappear with the soon to come migration to React.


client/components/boards/boardHeader.js, line 218 [r1] (raw file):
Hum, why do you have to use that? Isn’t something like this.currentData()._id.trim() supposed to work?


models/boards.js, line 347 [r1] (raw file):
I guess if the board is public you don’t need to be a member to clone it. We should abstract this condition somewhere, IIRC it’s already duplicated in two places (the publication function, and the export generation).


models/boards.js, line 399 [r1] (raw file):
As it is still possible to restore them, maybe we should copy archived lists as well?


models/boards.js, line 406 [r1] (raw file):
This is a bit weird to me, and sounds a bit error prone. Is this just some optimization or is there a deeper rationale?


models/boards.js, line 411 [r1] (raw file):
Do we need a $set here? If we want to just replace the entire document we can use .update(selector, newDocument)


Comments from the review on Reviewable.io

@mquandalle
Copy link

On the review process, I don’t really like the default way of batching all the comments in a single message, will see if I can configure so that it interfaces with GitHub inline comments.

@floatinghotpot
Copy link
Contributor Author

models/boards.js, line 411 [r1] (raw file):
The document are not replaced entirely. At least, the _id won't be changed, especially if there are some cards, the listId should not be changed.


Comments from the review on Reviewable.io

@mquandalle
Copy link

Reviewed 1 of 4 files at r1, 1 of 2 files at r2.
Review status: 3 of 4 files reviewed at latest revision, 4 unresolved discussions.


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

This review tool is not bad. Have a try. :-)

You are correct, cloning a empty board is importing template.

Will it be over-engineering if clone all cards (and attachments, comments)? My own use case is to setup a new board, and the needs in #308 discussion seems same.

And, a possible use case is, there are already some cards on current board, and user may wanna keep them, but apply template only.

Should give user the option to select what to clone: members, labels, lists, cards, attachments, comments?


Comments from the review on Reviewable.io

@mquandalle
Copy link

Review status: 3 of 4 files reviewed at latest revision, 5 unresolved discussions.


client/components/boards/boardHeader.jade, line 88 [r2] (raw file):
The id= argument isn’t needed anymore.


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

client/components/boards/boardHeader.jade, line 85 [r1] (raw file):
I had a try to use it, but encountered a error:
Error: Can't call non-function: undefined
at Spacebars.call (spacebars-runtime.js:136)
at Spacebars.mustacheImpl (spacebars-runtime.js:70)
at Object.Spacebars.dataMustache (spacebars-runtime.js:102)
at boardHeader.jade.js:319


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

client/components/boards/boardHeader.js, line 218 [r1] (raw file):
Ok, yes, it works.


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

models/boards.js, line 347 [r1] (raw file):
Ok. Only check private board now.


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

models/boards.js, line 399 [r1] (raw file):
Ok. Also copy archived lists now.


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

models/boards.js, line 406 [r1] (raw file):
The current board may have existing lists. And, they may even have cards on them. So need keep the lists, but copy the titles only.


Comments from the review on Reviewable.io

@mquandalle
Copy link

Review status: 3 of 4 files reviewed at latest revision, 6 unresolved discussions.


client/components/boards/boardHeader.jade, line 85 [r1] (raw file):
Ok I’m a bit surprised but that’s not important. Let’s keep it like what you have done (again, this will be re-factored with React anyway).


Comments from the review on Reviewable.io

@floatinghotpot
Copy link
Contributor Author

@floatinghotpot
Copy link
Contributor Author

@mquandalle
Copy link

Reviewed 1 of 1 files at r3.
Review status: 3 of 4 files reviewed at latest revision, 2 unresolved discussions.


Comments from the review on Reviewable.io

@shekar73
Copy link

@mquandalle Is this going to be merged? I though the code review is already done?

@mquandalle
Copy link

So first of all I have to apologize to @floatinghotpot because I did the code review before actually running the code on my machine as I was excited to test Reviewable (I’m happy with it BTW, it allows me to keep track of sub issues in a better way and it supports git rebasing). So I actually misunderstood this feature as being a “full clone” feature whereas it is actually a way to re-use a board template (similar to what is discussed in #308). This doesn’t change much in my review, just the fact that not copying archived lists as you originally did @floatinghotpot, was actually making sense.

Also I feel like the UX is not intuitive, because you first have to create a board and then go in the board menu to “Import template” (and if you already have some data in the board the UI will alert you that the board structure will be overwritten, “This action will overwrite current members, labels and lists.”). I think this feature makes much more sense as an option in the creation form:

Create Board popup

I’m not sure about the actual UI, I just modified the HTML in the browser tool to share the idea that the template should be defined on the board instantiation, not by overwriting any structure at a latter time.

Also I would really like this feature to be implemented as a plugin, maybe installed and activated by default, but the plugin work will rely on Meteor 1.3 that is currently not working with Wekan. Hopefully MDG will release a new beta this week so I can start writing a 0.1 version of the module system and API. If I can’t get that minimal version implemented in any reasonable delay, then I will merge this feature without the ideal modularity and that will come later.

@floatinghotpot
Copy link
Contributor Author

Yes, I thought of adding it when creating a new board, but did it in current ugly way in a hurry for our project needs.

I will wait your plugin system ready, then re-write it.

@irfanitw
Copy link

@mquandalle @floatinghotpot, this is a great feature! What do we need to get this thru?

@xet7 xet7 added this to the v0.12 milestone Feb 11, 2017
@xet7 xet7 changed the title add: clone board template (members, labels, lists) from another board Add Feature: Move/Clone board or list Apr 2, 2017
@FrDu
Copy link
FrDu commented Jul 10, 2017

How is the status of this project ?
Is there a time frame for to be added into a main release ?

@xet7
Copy link
Member
xet7 commented Jul 16, 2017

Related #979 and #797

@xet7 xet7 changed the title Add Feature: Move/Clone board or list Add Feature: Board templates Nov 23, 2017
@xet7 xet7 changed the title Add Feature: Board templates Add Feature: Board templates or other stuff Nov 23, 2017
@xet7 xet7 added this to the 2019-03-31 X milestone Jun 24, 2018
@GregBusch1995
Copy link

Status?

@xet7
Copy link
Member
xet7 commented Feb 27, 2019

@GregBusch1995

This old PR is not related to current Template issue #2165 that is not merged yet. Template issue will be added to stable release when it works.

@xet7
Copy link
Member
xet7 commented Feb 27, 2019

@GregBusch1995

Wekan v2.29 is now released, it includes current Templates feature that is in progress. If you find some new bugs, please add comment to #2165

@xet7 xet7 closed this May 14, 2019
@xet7 xet7 reopened this May 14, 2019
@justinr1234
Copy link
Contributor

@xet7 is this one still relevant?

@xet7
Copy link
Member
xet7 commented Jul 3, 2019

@justinr1234

No, Wekan already has board templates.

@xet7 xet7 closed this Jul 3, 2019
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.

8 participants
0