-
Notifications
You must be signed in to change notification settings - Fork 281
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
Clone cards together with the board #3430
Conversation
|
Restoring archived cards would in no way be an edge case! |
a0182bc
to
e53f9f4
Compare
Hello @bahuma20. I'm very sorry for the long delay on the feedback of this one, we'll have a look into it soonish to get this merged. |
lib/Service/BoardService.php
Outdated
@@ -80,11 +85,13 @@ class BoardService { | |||
public function __construct( | |||
BoardMapper $boardMapper, | |||
StackMapper $stackMapper, | |||
CardMapper $cardMapper, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 2 injections of CardMapper
, see below at line 97.
@@ -662,6 +672,16 @@ public function clone($id, $userId) { | |||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can separate the board cloning logics in a different class - The BoardMapper or the Service.
@@ -103,11 +107,13 @@ public function setUp(): void { | |||
$this->service = new BoardService( | |||
$this->boardMapper, | |||
$this->stackMapper, | |||
$this->cardMapper, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two CardMapper usages in the BoardService constructor.
Agree with @stefan-niedermann – we should use |
Yes but I'd say that |
This would be nice to have merged. This would solve a problem for a few projects where they have asked for this type of feature. |
8b4176a
to
1b87aed
Compare
I rebased this as there were quite some conflicting changes. Will have a look what is missing to get this in. |
88dda3b
to
eac0400
Compare
eac0400
to
b627884
Compare
4ce2d79
to
0473133
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cypress tests should be added.
We would also be very interested in this feature. |
I would also be very interested in this feature. Why it has not been merged yet? |
0473133
to
9e9ec2f
Compare
9e9ec2f
to
9b2c3f9
Compare
Signed-off-by: Max Bachhuber <[email protected]> Adjust BoardServiceTest to new dependencies Signed-off-by: Max Bachhuber <[email protected]> Add BoardCloneModal vue component to frontend. Adjust BoardApi and store to support clone options Signed-off-by: Max Bachhuber <[email protected]> Add license and credits Signed-off-by: Max Bachhuber <[email protected]> Fix PHP code style Signed-off-by: Max Bachhuber <[email protected]> Change default clone settings Signed-off-by: Max Bachhuber <[email protected]> Add accordion for advanced settings Signed-off-by: Max Bachhuber <[email protected]> Fix bug which caused board to be cloned when clicking out of the modal Signed-off-by: Max Bachhuber <[email protected]> Change wording of clone options Signed-off-by: Max Bachhuber <[email protected]> fix: Rebase failures Signed-off-by: Julius Härtl <[email protected]> update cloneBoards phpdoc make error message clear SPDX Header BoardCloneModal.vue Signed-off-by: grnd-alt <[email protected]>
9b2c3f9
to
f2c30af
Compare
Pushed some tests and fixed remaining review comments where reasonable. I'll get this merged for now even if we lack attachment and comment cloning. I think this is valuable enough without so we can get this unblocked. |
Summary
Follow Up
TODO
Checklist