search

nextcloud / user_external

👥 External user authentication methods like IMAP, SMB and FTP
https://apps.nextcloud.com/apps/user_external
107 stars 64 forks source link

Use new types introduced in Nextcloud 21 and bump compatibility #173

Closed hlnd closed 3 years ago

hlnd commented 3 years ago

Signed-off-by: Ole Morten om@haaland.biz

Fixes #165.

Changes proposed in this pull request:

violoncelloCH commented 3 years ago

Thanks for the PR @hlnd @ChristophWurst could you take a look if those changes make sense and if there is nothing missing? (That way I could hopefully create a new release next week after this is merged)

CI is failing because of #159 (which is already out of date again)

almereyda commented 3 years ago

After checking the patch, I think it might not be the best idea to change an existing migration.

On the other hand were the DBAL types removed with 21, and that migration couldn't possibly run anymore.

But will this patch also allow to take care of clients, which have existing users_external tables already initialized with Type::String, or is this kind of type migration taken care of by the NC21 upgrade migration?

ChristophWurst commented 3 years ago

The migration will still be run for new migrations. Existing migration files have to be touched.

doobry-systemli commented 3 years ago

I can verify that the app works as expected with Nextcloud 21.

Would be nice if this PR could be merged and a new app release be published @violoncelloCH :blush:

violoncelloCH commented 3 years ago

thanks for the PR @hlnd !

oriolo commented 3 years ago

Well done, It works as expected. Just two minor issues to report with this configuration: '', , '<ssl/tls>', ', true, true the user is created but no group is defined for him nor the email address. Just in case I would also like to suggest to add a 7th parameter in which one can define the quota limit. Thank you.

ychaouche commented 3 years ago

Thank you all. The red enable untested app has now disappeared from the admin pannel > apps.