Transfer ownership

[matchish]

• Resolves: New Owner for a board #1306
• Target version: master

Summary

Add deck:transfer-ownership command

TODO

• Add tests

Checklist

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

[matchish]

Before add tests I would like to have small feedback about the command.
Also I wondering it's ok if I add integration tests for the command instead of unit?
I don't see a way to create good unit tests for the command.

[stefan-niedermann]

Can you please also update the REST API documentation so 3rd party clients can implement this new feature? 🙂

[juliusknorr]

Thanks a lot for looking into this. ❤️

Before add tests I would like to have small feedback about the command.
Also I wondering it's ok if I add integration tests for the command instead of unit?

Sure, I agree that an integration test makes the most sense here.

[matchish]

Can you please also update the REST API documentation so 3rd party clients can implement this new feature?

It's an occ command. Not a REST API. What documentation I have to update?

[stefan-niedermann]

Oh - uhm... Basically to gain feature parity we would need a REST endpoint additionally then? Should i open a new issue then?

How does the vue app trigger an owner change? Doesn't it has to call some kind of REST endpoint, too? Or am i completely wrong here?

[matchish]

I believe this feature mostly for admins and they have access to occ commands. And if we want REST endpoint for the feature I think is better to open a new issue for it(for me smaller tasks always better)

[matchish]

Ready for review again) There is one issue. If user already participant of some card or in acl then command will stop with error unique constrain violation. Do you think this could be a new task(issue), because this implementation already has an value?

[juliusknorr]

Sorry for taking so long to review.

There is one issue. If user already participant of some card or in acl then command will stop with error unique constrain violation. Do you think this could be a new task(issue), because this implementation already has an value?

I'd love to get this addressed before merge, since that is probably a common use case that the ownership is transferred to a user that is already part of the board. I'm not sure if there is an easy solution without manually iterating over the entries and update them one by one.

MySQL would support an UPDATE IGNORE statement modifier but this doesn't seem to be available in PostgreSQL or sqlite so that is not an option for us here.

[juliusknorr]

The code in general looks good to me 👍

[juliusknorr]

Lint / php-cs check (pull_request) Failing after 34s — php-cs check

[juliusknorr]

The BoardServiceTest needs to be adjusted to the additional constructor parameter

[matchish]

The BoardServiceTest needs to be adjusted to the additional constructor parameter

I'm not sure I understand you. What do you mean?

[matchish]

@juliushaertl There is failed check https://drone.nextcloud.com/nextcloud/deck/4233/2/2
Could you help me with debug, don't understand how to reproduce it locally? All my tests pass locally

[juliusknorr]

Sorry for the delay, unfortunately I totally missed your message. I've rebased and pushed it to a branch of this repo which makes collaborating on the branch a bit easier #2496

Since you are a member of the nextcloud organization you should also have push access to that.