Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Revert formatting of sync_sso groups to match API #257

Closed
wants to merge 2 commits into from

Conversation

bgastelo
Copy link

After upgrading from 0.47.0. to 0.48.1, I noticed our sync_sso calls started failing with 500s from Discourse. Reviewing the Discourse logs showed that it expects groups to be a comma separated string, not an array. This line here.

This is also mentioned in the setup doc:

If the discourse connect overrides groups option is specified, Discourse will consider the comma separated list of groups passed in groups.

I am running an older version of discourse (2.8.13) but I do not believe that this API endpoint has changed in new versions.

The PR that introduced this change was trying to modify DiscourseApi::SingleSignOn#groups, but changed both the #parse and #parse_hash methods. I believe #parse_hash is used exclusively for calling the sync_sso from discourse. Looks like an accidental change.

I am also modifying the spec to check the groups option as well. So this behavior is not accidentally changed again.


If I'm wrong and there was an API change, feel free to ignore this PR.

Thanks

@bgastelo bgastelo closed this Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant