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.

Sign up for GitHub

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

Jump to bottom

feat(cards): add card cloning ability #6452

Merged
merged 12 commits into from
Dec 19, 2024
Merged

feat(cards): add card cloning ability #6452

merged 12 commits into from
Dec 19, 2024

Conversation

grnd-alt
Copy link
Member

@grnd-alt grnd-alt commented Oct 29, 2024 •
edited
Loading

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@grnd-alt grnd-alt linked an issue Oct 29, 2024 that may be closed by this pull request
Clone/copy a card #1813
Closed
@grnd-alt grnd-alt force-pushed the feat/1813-clonecopy-a-card branch from 950429a to 4212c10 Compare November 4, 2024 11:58
@grnd-alt grnd-alt requested a review from juliusknorr November 7, 2024 09:51
@luka-nextcloud
Copy link
Contributor

@grnd-alt Missing unit test for function CardService->cloneCard()
Missing unit test for function LabelService->cloneLabelIfNotExists()
I think cypress tests should be added as well.

luka-nextcloud
luka-nextcloud previously requested changes Nov 7, 2024
View reviewed changes
Copy link
Contributor

@luka-nextcloud luka-nextcloud left a comment

Choose a reason for hiding this comment

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

Please review tests. Otherwise it looks good!

@grnd-alt grnd-alt force-pushed the feat/1813-clonecopy-a-card branch from 3045c71 to d5c2621 Compare November 14, 2024 09:49
@hectorandac
Copy link

This is awesome! I'm eagerly waiting for this feature 🎉

@grnd-alt grnd-alt requested a review from elzody as a code owner November 25, 2024 12:56
@grnd-alt grnd-alt force-pushed the feat/1813-clonecopy-a-card branch from cdc07d8 to cbb13e2 Compare November 25, 2024 13:03
grnd-alt and others added 8 commits December 18, 2024 22:31
@grnd-alt @juliusknorr
feat(cards): add card cloning ability
e436cb9 
Signed-off-by: grnd-alt <[email protected]>
@grnd-alt @juliusknorr
fix(cards): use copy-content icon for cloning cards
55c44ef 
Signed-off-by: grnd-alt <[email protected]>
@grnd-alt @juliusknorr
introduce cloneCard backendFunctionality
838b648 
Signed-off-by: grnd-alt <[email protected]>
@grnd-alt @juliusknorr
check user-permsisions when assigning to cloned card
1fe20dc 
Signed-off-by: grnd-alt <[email protected]>
@grnd-alt @juliusknorr
feat(tests): add tests for labelService and cardService functions
2e0f0d2 
Signed-off-by: grnd-alt <[email protected]>
@grnd-alt @juliusknorr
add cypress tests for cardfeatures
0a48954 
Signed-off-by: grnd-alt <[email protected]>
@grnd-alt @juliusknorr
skip cypress test causing chrome out of memory crash
d829a28 
Signed-off-by: grnd-alt <[email protected]>
@juliusknorr
tests(cypress): Try sleep to avoid electron crash in modal
3391a40 
Signed-off-by: Julius Knorr <[email protected]>
@juliusknorr juliusknorr force-pushed the feat/1813-clonecopy-a-card branch from ed2f7b5 to 3391a40 Compare December 18, 2024 21:59
@juliusknorr
Copy link
Member

@grnd-alt I pushed two commits to get CI passing again, the crashing electron process is a mystery still to me but seems giving it a bit of time before typing (likely to animate the modal showing) solves that.

Generally I've tested and works fine, will have another look at the code.

juliusknorr
juliusknorr reviewed Dec 18, 2024
View reviewed changes
appinfo/routes.php Outdated Show resolved Hide resolved
juliusknorr
juliusknorr reviewed Dec 18, 2024
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Some comments inline, generally looks good to me

@grnd-alt grnd-alt force-pushed the feat/1813-clonecopy-a-card branch from 76d11ad to 22bb8d6 Compare December 19, 2024 12:39
@juliusknorr
Copy link
Member

Did one small commit as i thought we could have a quick refactoring to rather use NcDialog instead of NcModal

@juliusknorr
Copy link
Member

Pending documentation as we should add the new endpoint to docs/API.md

@juliusknorr juliusknorr merged commit fb236ed into main Dec 19, 2024
37 checks passed
@juliusknorr juliusknorr deleted the feat/1813-clonecopy-a-card branch December 19, 2024 15:26
@juliusknorr juliusknorr mentioned this pull request Jan 2, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliusknorr juliusknorr juliusknorr approved these changes

@elzody elzody Awaiting requested review from elzody elzody is a code owner

@luka-nextcloud luka-nextcloud Awaiting requested review from luka-nextcloud luka-nextcloud is a code owner

Assignees
No one assigned
Labels
3. to review enhancement pending documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clone/copy a card
4 participants
@grnd-alt @luka-nextcloud @hectorandac @juliusknorr