Skip to content

Conversation

@bassepeder
Copy link
Contributor

@bassepeder bassepeder commented Jul 27, 2024

Closes #15761.

This change should not be taking lightly as it will affect all API responses using this function.

To put it short, e.args is a Seq[Matchable]. Without these changes, toString() gets called which is causing this to output ArraySeq(30), where 30 is the element in the sequence. If the sequence contained more elements, such as 60, it would ouput ArraySeq(30, 60).

By doing mkString it will concatenate all elements of the sequence into a string. We might want to do mkString(", ") to comma-separate each element if there are more than one. Going back to my example of ArraySeq(30, 60), the output would look like Must be at least 30, 60 characters long. That output does not make sense in this context, but its worth thinking about for other usages.

CleanShot 2024-07-27 at 12 49 13

@ornicar
Copy link
Collaborator

ornicar commented Jul 27, 2024

.txt takes multiple arguments. https://github.com/lichess-org/lila/blob/master/modules/coreI18n/src/main/key.scala#L10

I don't think it makes sense to concatenate the arguments into one string. Instead, all arguments should be passed to .txt.
Try something like e.args* or whatever the scala syntax for that is.

@kraktus
Copy link
Member

kraktus commented Jul 27, 2024

I think it's the weird e.args: _* thing

@bassepeder
Copy link
Contributor Author

bassepeder commented Jul 27, 2024

.txt takes multiple arguments. https://github.com/lichess-org/lila/blob/master/modules/coreI18n/src/main/key.scala#L10

I don't think it makes sense to concatenate the arguments into one string. Instead, all arguments should be passed to .txt. Try something like e.args* or whatever the scala syntax for that is.

I have made the change to use vararg expansion with e.args*, and confirmed that it works as expected.

@ornicar ornicar merged commit da837f4 into lichess-org:master Jul 28, 2024
@bassepeder bassepeder deleted the fix-team-join-validation-response branch October 16, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix API validation response for team join message

3 participants