vikunja
/
vikunja
6
43
Fork 71
Code Issues 150 Pull Requests 15 Packages Releases 32 Activity

Trello: Include non upload attachments #2261

Merged
konrad merged 6 commits from Elscrux/vikunja:feature/trello-non-upload-attachements into main 2024-04-10 22:12:09 +00:00
Conversation 2 Commits 6 Files Changed 2 +43 -33
Elscrux commented 2024-04-09 16:52:08 +00:00
Contributor
Copy Link

This makes the Trello migrator include attachments that are not file uploads. To include them in Vikunja without missing data, their text (usually links) will be prepended to the Vikunja description.

This code was already tested in a migration and the unit tests were also extended.

This makes the Trello migrator include attachments that are not file uploads. To include them in Vikunja without missing data, their text (usually links) will be prepended to the Vikunja description. This code was already tested in a migration and the unit tests were also extended.
Elscrux added 1 commit 2024-04-09 16:52:10 +00:00
continuous-integration/drone/pr Build is passing Details
a84f7a5a7f Trello: Include non upload attachments 
This makes the Trello migrator include attachments that are not file uploads. To include them in Vikunja without missing data, their text (usually links) will be prepended to the Vikunja description.
frederick commented 2024-04-09 16:58:39 +00:00
Member
Copy Link

Hi Elscrux!

Thank you for creating a PR!

I've deployed the frontend changes of this PR on a preview environment under this URL: https://2261-feature-trello-non-upload-attach--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/.

This preview does not contain any changes made to the api, only the frontend.

Have a nice day!

Beep boop, I'm a bot.

Hi Elscrux! Thank you for creating a PR! I've deployed the frontend changes of this PR on a preview environment under this URL: https://2261-feature-trello-non-upload-attach--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somewhere. The easiest to use is https://try.vikunja.io/. This preview does not contain any changes made to the api, only the frontend. Have a nice day! > Beep boop, I'm a bot.
konrad requested changes 2024-04-09 20:50:27 +00:00
Dismissed
pkg/modules/migration/trello/trello.go Outdated
@ -342,0 +364,4 @@
task.Attachments = append(task.Attachments, vikunjaAttachment)
log.Debugf("[Trello Migration] Downloaded card attachment %s", attachment.ID)
} else {
konrad commented 2024-04-09 20:50:24 +00:00
Owner
Copy Link

Nitpick: please use a continue here and get rid of the else. Makes it more idiomatic.

Nitpick: please use a `continue` here and get rid of the `else`. Makes it more idiomatic.
konrad marked this conversation as resolved
pkg/modules/migration/trello/trello.go Outdated
@ -342,0 +366,4 @@
log.Debugf("[Trello Migration] Downloaded card attachment %s", attachment.ID)
} else {
// Other links are not attachments in Vikunja, but we can add them to the description
task.Description = "<p><a href=\"" + attachment.URL + "\">" + attachment.URL + "</a></p>\n" + task.Description
konrad commented 2024-04-09 20:46:47 +00:00
Owner
Copy Link

Please change this so that the attachments are appended to the description, not added in front of the rest.

Please also refactor the string concatenation to something like

`<p><a href="` + attachment.URL + `">` + attachment.Name + `</a></p>`+"\n"
Please change this so that the attachments are appended to the description, not added in front of the rest. Please also refactor the string concatenation to something like ``` `<p><a href="` + attachment.URL + `">` + attachment.Name + `</a></p>`+"\n" ```
konrad marked this conversation as resolved
Elscrux added 2 commits 2024-04-10 12:03:15 +00:00
continuous-integration/drone/pr Build is failing Details
5cb8179fa3 Append attachments instead of prepending
Elscrux added 1 commit 2024-04-10 12:04:28 +00:00
continuous-integration/drone/pr Build is passing Details
0e93ec1529 Fix test
konrad reviewed 2024-04-10 21:35:24 +00:00
pkg/modules/migration/trello/trello.go Outdated
@ -368,2 +370,2 @@
log.Debugf("[Trello Migration] Downloaded card attachment %s", attachment.ID)
// Other links are not attachments in Vikunja, but we can add them to the description
task.Description += `<p><a href="` + attachment.URL + `">` + attachment.URL + "</a></p>\n"
konrad commented 2024-04-10 21:35:24 +00:00
Owner
Copy Link

Please use attachment.Name as the link text here.

Please use `attachment.Name` as the link text here.
konrad commented 2024-04-10 21:35:48 +00:00
Owner
Copy Link

Or maybe use the url as fallback, but if the attachment has a name that should be preferred.

Or maybe use the url as fallback, but if the attachment has a name that should be preferred.
👍 1
konrad marked this conversation as resolved
Elscrux added 1 commit 2024-04-10 21:43:09 +00:00
continuous-integration/drone/pr Build is failing Details
8b5f20da4a Use name for display
konrad approved these changes 2024-04-10 21:55:32 +00:00
konrad left a comment
Owner
Copy Link

Thanks!

Thanks!
konrad scheduled this pull request to auto merge when all checks succeed 2024-04-10 21:56:03 +00:00
konrad commented 2024-04-10 21:56:32 +00:00
Owner
Copy Link

Please fix the test.

Please fix the test.
Elscrux added 1 commit 2024-04-10 21:59:15 +00:00
continuous-integration/drone/pr Build is passing Details
036a188a23 Fix test
konrad merged commit 61ee0bd5e2 into main 2024-04-10 22:12:09 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
konrad
Labels
Clear labels   
dependencies

   
duplicate

This issue or pull request already exists

  
help wanted

Need some help

  
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
needs reproduction

   
question

More information is needed

  
security

   
wontfix

This won't be fixed

No Label
dependencies
duplicate
help wanted
invalid
kind/bug
kind/feature
needs reproduction
question
security
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/vikunja#2261
No description provided.
Delete Branch "Elscrux/vikunja:feature/trello-non-upload-attachements"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?