Skip to content

Comments

add migrator for nextcloud deck app#1823

Draft
kermorgant wants to merge 10 commits intogo-vikunja:mainfrom
kermorgant:feat/nextcloud-deck-migrator
Draft

add migrator for nextcloud deck app#1823
kermorgant wants to merge 10 commits intogo-vikunja:mainfrom
kermorgant:feat/nextcloud-deck-migrator

Conversation

@kermorgant
Copy link

@kermorgant kermorgant commented Nov 15, 2025

This PR is the result of asking claude code to write a migrator for nextcloud deck. I'm afraid it might hurt the eyes when reviewing, but thought it might still be worth sharing.

I managed to successfully import a dozen of boards from my self hosted nextcloud 30 instance, with attachments and comments.

I should be able to fix things in the near future, but can't promise any long term commitment.

UX

Picking the migrator

(it is always enabled : there's no need for the admin to configure anything in the config.yml, so maybe it can be shown here unconditionally ?)
image

Configuring

(the redirect url is probably useless, since it seems to be enforced by nextcloud)

image

Copy link
Member

@kolaente kolaente left a comment

Choose a reason for hiding this comment

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

Thanks for being upfront about Claude. This does need more work if we want to integrate it.

Another option would be to rework this so that it can handle a nextcloud export? That would be possible with less changes to the migrator structure.


// For POST requests, bind the request body to the migrator struct
// This allows migrators that need server URL to receive it
if c.Request().Method == http.MethodPost {
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of binding the request body and then not doing anything with it?

t.BucketID = 0
err = t.Create(s, user)

// Debug logging for datetime preservation
Copy link
Member

Choose a reason for hiding this comment

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

this should be removed

}

// Nextcloud Deck
// Note: Unlike other OAuth migrators, Deck is always registered because it requires
Copy link
Member

Choose a reason for hiding this comment

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

If it's alwasy registered, the config should default to true

Copy link
Author

Choose a reason for hiding this comment

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

That was a mistake. The config has a boolean to enable the migrator, so I'll change the code

config-raw.json Outdated
"comment": "Whether to enable the Nextcloud Deck migrator."
},
{
"key": "clientid",
Copy link
Member

Choose a reason for hiding this comment

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

But the point of the new structure is to add the nextcloud login url in the UI? Why do I need to provide a client id and secret in the config?

Copy link
Author

Choose a reason for hiding this comment

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

True, this was not thought through. I guess this PR would be simpler if we put everything in the config ? I'll probably give this a shot unless you see this differently ?

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of letting users migrate from any instance. That would mean users should be able to enter everything themselves, or just provide an exported file.

Copy link
Author

@kermorgant kermorgant Nov 15, 2025

Choose a reason for hiding this comment

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

agree, that's better. I'll try. But I give up with exported file. I figured I had to install an app (on NC), but once installed there was some issue...

@kolaente
Copy link
Member

Can you explain how the flow works in general?

@kermorgant
Copy link
Author

Thanks for being upfront about Claude. This does need more work if we want to integrate it.

Sure, that was the least I could do. I took a deliberate position of not trying to understand the codebase when starting that attempt (I was ready to give up if that did not work, and I have limited time & energy for such side work, just to give a bit of my context). But I got something working and thanks to your feedback, it feels like this could be worth a bit more effort :-)

Another option would be to rework this so that it can handle a nextcloud export? That would be possible with less changes to the migrator structure.

That was something I considered at the beginning but Claude warned against this being a dead end for getting attachments. Not sure how much truth there's in that, but that led me where we are here :-)

Mikael Kermorgant added 2 commits November 15, 2025 17:29
* remove lots of debug logs
* remove api doc changes
* enable migrator according to flag in config file
nextcloud migrator settings 100% provided by user
no need for config file settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants