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

export json data of commments #6119

Merged
merged 1 commit into from
Aug 5, 2024
Merged

export json data of commments #6119

merged 1 commit into from
Aug 5, 2024

Conversation

grnd-alt
Copy link
Member

@grnd-alt grnd-alt commented Jul 19, 2024 •
edited
Loading

Summary

add comments to json export

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 Jul 19, 2024 that may be closed by this pull request
Comment json dump #6118
Closed
@grnd-alt grnd-alt force-pushed the feat/6118-comment-json-dump branch 2 times, most recently from 55fe228 to 9ef51a0 Compare July 19, 2024 10:25
@juliusknorr
Copy link
Member

juliusknorr commented Jul 23, 2024 •
edited
Loading

Code looks good, tests need some adjustment:

There was 1 error:

1) OCA\Deck\Command\UserExportTest::testExecute
TypeError: OCA\Deck\Command\UserExport::__construct(): Argument #7 ($commentService) must be of type OCA\Deck\Service\CommentService, Mock_IUserManager_f15701a4 given, called in /home/runner/actions-runner/_work/deck/deck/apps/deck/tests/unit/Command/UserExportTest.php on line 62

juliusknorr
juliusknorr requested changes Jul 23, 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.

One remark/question

lib/Service/Importer/Systems/DeckJsonService.php
return [];
$comments = [];
foreach ($this->tmpCards as $sourceCard) {
$commentsOriginal = $sourceCard->comments;
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to handle cases where comments is not defined or null here? I could imagine this breaking otherwise if an old json without comments is used?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we have to. The next loop in line 113, for my understanding, just gets skipped if $commentsOriginal is not iterable. I tested it without comments in the json and it worked without any errors.

@grnd-alt grnd-alt force-pushed the feat/6118-comment-json-dump branch 3 times, most recently from 153953d to 655a922 Compare August 1, 2024 11:04
@grnd-alt grnd-alt requested a review from juliusknorr August 5, 2024 07:23
@juliusknorr juliusknorr force-pushed the feat/6118-comment-json-dump branch from 655a922 to 09748ae Compare August 5, 2024 08:05
@grnd-alt grnd-alt merged commit 5153135 into main Aug 5, 2024
32 checks passed
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

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud

Assignees
No one assigned
Labels
2. developing enhancement feature: export/import
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Comment json dump
2 participants
@grnd-alt @juliusknorr