Skip to content

feat: add saml:user:add command to pre-provision users#962

Merged
blizzz merged 10 commits into
nextcloud:masterfrom
guerby:patch-user-add-fix-939
Jul 29, 2025
Merged

feat: add saml:user:add command to pre-provision users#962
blizzz merged 10 commits into
nextcloud:masterfrom
guerby:patch-user-add-fix-939

Conversation

@guerby

@guerby guerby commented May 20, 2025

Copy link
Copy Markdown
Contributor

feat: add saml:user:add command to pre-provision users, fix #939

@guerby
guerby requested a review from blizzz as a code owner May 20, 2025 12:34
@blizzz blizzz changed the title fix 939 feat: add saml:user:add command to pre-provision users May 23, 2025

@blizzz blizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for your contribution!

Some suggestions on user facing text mostly. Please update the commit message to be meaningful, e.g. how i edited the PRs title.

Comment thread lib/Command/UserAdd.php Outdated
Comment thread lib/Command/UserAdd.php Outdated
Comment thread lib/Command/UserAdd.php Outdated
Comment thread lib/Command/UserAdd.php Outdated
@guerby
guerby force-pushed the patch-user-add-fix-939 branch 2 times, most recently from fd0c258 to a68adae Compare May 23, 2025 12:02
@github-actions

github-actions Bot commented Jun 4, 2025

Copy link
Copy Markdown

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@blizzz

blizzz commented Jun 11, 2025

Copy link
Copy Markdown
Member

FWIW Github does not create notifications when a commit was added.

@guerby

guerby commented Jun 16, 2025

Copy link
Copy Markdown
Contributor Author

FWIW Github does not create notifications when a commit was added.

I have commited the change to make email VALUE_OPTIONAL, I think this is the last pending review item, let me know if I missed something, thanks!

@blizzz

blizzz commented Jun 19, 2025

Copy link
Copy Markdown
Member

Thanks! Please update with a rebase, not merging master hereinto.

@guerby
guerby force-pushed the patch-user-add-fix-939 branch from 030d421 to 55a16f6 Compare June 19, 2025 11:05
@guerby

guerby commented Jun 19, 2025

Copy link
Copy Markdown
Contributor Author

Thanks! Please update with a rebase, not merging master hereinto.

I have followed the DCO suggestions:

git pull
git rebase HEAD~9 --signoff
git push --force-with-lease origin patch-user-add-fix-939

I hope this is correct :)

@guerby
guerby force-pushed the patch-user-add-fix-939 branch 2 times, most recently from 1265555 to 61f344c Compare June 19, 2025 11:12
@blizzz

blizzz commented Jun 20, 2025

Copy link
Copy Markdown
Member

I use git rebase master instead of git rebase HEAD~9 but it looks good anyhow ✔️

@blizzz

blizzz commented Jun 20, 2025

Copy link
Copy Markdown
Member

composer i && composer run cs:fix should help solve the last failing test.

Plusses, if the UPDATE $filename commits can be squashed to their parent :)

@guerby

guerby commented Jun 20, 2025

Copy link
Copy Markdown
Contributor Author

composer i && composer run cs:fix

On my debian 12 I did apt-get install composer php-xml php-curl and ran composer i && composer run cs:fix.

For the squash part I'm reading git documentation, I assume you mean commits 29ba794 2cced28 and b886e05 ?

@blizzz

blizzz commented Jun 23, 2025

Copy link
Copy Markdown
Member

composer i && composer run cs:fix

On my debian 12 I did apt-get install composer php-xml php-curl and ran composer i && composer run cs:fix.

Thanks!

For the squash part I'm reading git documentation, I assume you mean commits 29ba794 2cced28 and b886e05 ?

Aye 🙂

@guerby
guerby force-pushed the patch-user-add-fix-939 branch from 316b67f to a93f4f8 Compare June 24, 2025 05:30
@guerby

guerby commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

composer i && composer run cs:fix

On my debian 12 I did apt-get install composer php-xml php-curl and ran composer i && composer run cs:fix.

Thanks!

For the squash part I'm reading git documentation, I assume you mean commits 29ba794 2cced28 and b886e05 ?

Aye 🙂

Hi, I think I've done the squash and fix whitespace that composer missed. The other lint errors are not in my code AFAIK.

Let me know if it look ok to you and thanks again!

@blizzz

blizzz commented Jun 24, 2025

Copy link
Copy Markdown
Member

i think something went wrong, the changes in the l10n should not be here though (they will be imported automatically from the translation tool)

@guerby

guerby commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

i think something went wrong, the changes in the l10n should not be here though (they will be imported automatically from the translation tool)

Do you have a suggestion on what to do to fix this PR (remove unwanted changes dragged from probably one of my git commands)?

@guerby
guerby force-pushed the patch-user-add-fix-939 branch from a93f4f8 to d1d9dbb Compare June 24, 2025 16:59
@guerby

guerby commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

i think something went wrong, the changes in the l10n should not be here though (they will be imported automatically from the translation tool)

I did the following to get rid of the unwanted commits:

git rebase -i HEAD~20 # removed the "pick" lines of all unwanted commits
git push origin patch-user-add-fix-939 --force-with-lease

They seem to have disappeared now.

@guerby
guerby force-pushed the patch-user-add-fix-939 branch 2 times, most recently from df159b1 to b2f1c0e Compare June 24, 2025 17:01
@guerby

guerby commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

Hmm they seem to reappear immediately, may be some bot is doing something?

@guerby
guerby force-pushed the patch-user-add-fix-939 branch from b2f1c0e to 53f767a Compare June 24, 2025 17:07
@blizzz

blizzz commented Jun 25, 2025

Copy link
Copy Markdown
Member

Hmm they seem to reappear immediately, may be some bot is doing something?

Is your master checkout up to date?

Comment thread lib/Command/UserAdd.php Outdated
@guerby
guerby force-pushed the patch-user-add-fix-939 branch from 53f767a to 4866e72 Compare July 18, 2025 15:21
@guerby

guerby commented Jul 18, 2025

Copy link
Copy Markdown
Contributor Author

@blizzz do you have a suggestion on how to make progress?

I did again the following:

git pull origin master --rebase
git push origin patch-user-add-fix-939 --force-with-lease

@guerby

guerby commented Jul 18, 2025

Copy link
Copy Markdown
Contributor Author

Then I cliked on github web "merge" button

guerby and others added 8 commits July 28, 2025 20:19
…#939

Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Imrove user message

Co-authored-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
Signed-off-by: Laurent GUERBY <laurent@guerby.net>
@guerby
guerby force-pushed the patch-user-add-fix-939 branch from 1f9a14e to 578f7df Compare July 28, 2025 18:19
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz

blizzz commented Jul 29, 2025

Copy link
Copy Markdown
Member

added a commit with just code style changes. it seems the cs check workflow also updates the dependencies, so that it runs php-cs-fixer in a different version than otherwise is installed.

Co-authored-by: Erwan Colin <zephone@protonmail.com>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
blizzz merged commit f0712d8 into nextcloud:master Jul 29, 2025
49 checks passed
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.

Add saml:user:add command to pre-provision users

3 participants