Fix identity and user retrieval when special characters are used

Fixes #4023 (closed)

I also added tests to make sure the user with special characters in his name is returned correctly.

@rspeicher this probably should be added to 8.3 as a patch.

mentioned in issue #4245 (closed)

• Since these special_* values are only used in this test, let's move them from the lets directly into the body of the test and avoid [mystery guests](http://xunitpatterns.com/Obscure%20Test.html#Mystery Guest).
• Once they're inlined, break up the testing phases with newlines.
• I know you were matching the style of the other tests, but we should avoid describe-ing symbols. Since it's a class method it should be describe '.find_by_uid_and_provider'.

I wanted to write it that way, but since the rest of the tests were written on the other way, I though, maybe there is a reason for it. I'll change it.

Regarding the lets if I move them to the body of the test, they will need to be properly declared without the let, right?

Right, just make them plain ol' Ruby local variables.

I could also leave them as lets if I write them before it and after the describe '.find_by_uid_and_provider'. Which would be better?

We don't have a standard for this right now, and certainly have leaned heavily on lets in the past, but my current preference here would be having everything the test uses defined within the context of the test. And since this is a single test case and there's no reusing of the variables, I think lets are overkill here.

Makes sense. i will write them as local variables for the test. Thanks for the review!

Added 1 commit:

• 1d3889eb - Fix identity and user retrieval when special characters are used

@patricio I think that looks much better.

Want to get this into 8.3.1, on the 24th.

Me too. I hope @DouweM can review this tomorrow on his morning, so that we can get in on the patch for our morning.

Added 1 commit:

• 662aa8ec - No mb_chars needed anymore

Enabled an automatic merge when the build for 662aa8ec succeeds

@patricio Looks good. I made one last change and enabled an automatic merge.

Status changed to merged

mentioned in commit 835333c4

Picked into 8-3-stable.

mentioned in commit e9a2ab11