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.

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

Import from Trello #3182

Merged
merged 21 commits into from
Dec 30, 2021
Merged

Conversation

vitormattos
Copy link
Contributor

@vitormattos vitormattos commented Jul 10, 2021

Summary

Command to import json from Trello boards to Deck.

PS: The import command is ready to accept import from other systems and not just Trello. To create import from other systems it will be necessary to create a class for the system to be imported.

Command help:

deck:import --help
Description:
  Import data

Usage:
  deck:import [options]

Options:
      --system=SYSTEM   Source system for import. Available options: Trello API, Trello JSON.
      --config=CONFIG   Configuration json file. [default: "config.json"]
      --data[=DATA]     Data file to import. [default: "data.json"]

API documentation
See docs/API.md

TODO

  • Import flow
    • Import board
    • Import labels
    • Import stacks
    • Import cards
    • Associate cards to labels
    • Import comments
    • Import attachments. Attachment URL added in the attachment table;
    • Import user assigment
      • Relationship of Trello users to Nextcloud users
  • Import systems
    • Import from Trello JSON (limited to 1000 Trello actions)
    • Import from Trello API (no data limits to import)
    • Implement architecture to make possible create an importer to other systems
  • Implement endpoints to import using Deck API
    • Endpoint to import using API
    • Endpoint to get available systems to import
    • Endpoint to get json schema of a system to import

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

@vitormattos vitormattos marked this pull request as draft July 10, 2021 14:38
@vitormattos vitormattos force-pushed the import-from-trello branch 5 times, most recently from 91e0fbe to 65cd36c Compare July 11, 2021 04:49
@vitormattos vitormattos marked this pull request as ready for review July 11, 2021 04:50
@vitormattos vitormattos marked this pull request as draft July 12, 2021 15:16
@vitormattos vitormattos force-pushed the import-from-trello branch 4 times, most recently from 38fb15c to 9f612f4 Compare July 16, 2021 04:40
@juliushaertl juliushaertl self-requested a review July 16, 2021 10:26
@vitormattos vitormattos marked this pull request as ready for review July 16, 2021 22:50
@vitormattos vitormattos marked this pull request as draft July 16, 2021 22:52
@vitormattos vitormattos marked this pull request as ready for review July 17, 2021 17:17
@vitormattos
Copy link
Contributor Author

Finally done!

Code coverage with tests is not ideal but I think it is acceptable.

The breaking tests are related to the setup of the server, it seems to me it has no relationship with this PR.

NOTE: It seems a good thing to remove support for PHP 7.2 and 7.3, I had to remove the beautiful arrow functions and heredoc.

@vitormattos
Copy link
Contributor Author

Hi @juliushaertl,

I organized the code and I think it is now more beautiful and ready to accept other import sources.

I didn't implement the frontend in VueJS for importing but it already has the API endpoints ready to be consumed and working.

I thought about implementing Asana import but I've committed too much in this PR :-D

@juliushaertl
Copy link
Member

juliushaertl commented Jul 23, 2021

First of all thanks a lot for your contribution, this is really awesome and looks very promising from a first quick look.

It might take a bit until I get to fully review it, but I'll dig into it in detail as soon as possible.

NOTE: It seems a good thing to remove support for PHP 7.2 and 7.3, I had to remove the beautiful arrow functions and heredoc.

We basically stick with what is compatible in the Nextcloud release, which is currently requiring 7.3 as a minimum PHP version. We can indeed drop 7.2 for any newer branch than stable1.2 which still needs it for Nextcloud 20.

@vitormattos
Copy link
Contributor Author

@juliushaertl to make code review easier, I updated the documentation with the class diagram.

I used the extension yUML for the VSCode|Codium.

@vitormattos vitormattos marked this pull request as draft July 24, 2021 22:03
@vitormattos vitormattos force-pushed the import-from-trello branch 2 times, most recently from 1a409a8 to a27fc62 Compare July 27, 2021 06:05
@vitormattos vitormattos marked this pull request as ready for review July 27, 2021 06:08
@vitormattos
Copy link
Contributor Author

Hi @juliushaertl,

I returned to draft to implement import using Trello API.
It's ready for code review again, I updated the documentation too.

Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Great work!

Could you provide a test JSON file to import?

If I understand correctly, when importing from Trello API, all data is first loaded and stored in BoardImportTrelloApiService attributes. Once everything is loaded, Deck entities are added into the DB. I think there is a chance to reach the max RAM usage when importing big Trello projects.

I don't know how hard it would be to make those changes but: Would it be possible to import progressively while the data is loaded?
It would be very effective in saving RAM usage, even more thanks to the request pagination. Data could be added after each doRequest call.

lib/Controller/BoardImportApiController.php Show resolved Hide resolved
lib/Service/BoardImportCommandService.php Outdated Show resolved Hide resolved
lib/Service/BoardImportTrelloApiService.php Outdated Show resolved Hide resolved
Check available helpers
Default permission: view only
Moved validate setting from helper to command
Turn more easy create a importer
Docblock and improvements on interface
lcfirst on system property
Helper moved to ImporHelper folder
Moved fixtures to ImportHelper
Rename settings to config
Big refactor to move import methods to service

Signed-off-by: Vitor Mattos <vitor@php.rio>
Big refactor to create route
Import participants

Signed-off-by: Vitor Mattos <vitor@php.rio>
Clean code
Clean attachment table

Signed-off-by: Vitor Mattos <vitor@php.rio>
Update documentation
Start implementing getSystems route
Code to route getSystems
Controller to board import
Change return
Increase coverage

Signed-off-by: Vitor Mattos <vitor@php.rio>
Fix visibility
Make compatible with php 7.2
Remove returing instance
Increase coverage
Reduce psalm info
Throw exception if system not defined
Increment coverage

Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Fixes on getBoard tests
Refactor
Reduce psalm info
Refactor to implement pattern
Change order of methods to put all abstract first and all public first

Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Implement name of system to import
Implement need validate data
Fix allowed system list
Start implementing Trello API service

Signed-off-by: Vitor Mattos <vitor@php.rio>
Validate get boad
change pattern of api params
Import only one board by api
Populate data from api
Update class diagram
Update documentation
Add return when success
Sort comments
Fix order of cards
Instructions of attachments

Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Co-authored-by: Julien Veyssier <eneiluj@posteo.net>

Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
@juliushaertl
Copy link
Member

Rebased and resovled the composer.lock conflict

@juliushaertl juliushaertl merged commit b58913e into nextcloud:master Dec 30, 2021
@juliushaertl
Copy link
Member

Great work, thanks a lot @vitormattos 👏

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.

Export/import [$30]
4 participants