Submitty icon indicating copy to clipboard operation
Submitty copied to clipboard

[Feature:API] Allow special latin characters in user names

Open skara9 opened this issue 2 years ago • 3 comments

What is the current behavior?

First and last names with special Latin alphabet letters, such as letters with accents or umlauts, are rejected by user validation. Resolves #8466

What is the new behavior?

The validation for User first and last names has been updated to include a subset of Latin-1 characters. This means names with these letters will no longer cause issues with user accounts on Submitty.

skara9 avatar Oct 01 '22 22:10 skara9

Codecov Report

Merging #8467 (cc39ac2) into main (7258d08) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head cc39ac2 differs from pull request most recent head ea09a69. Consider uploading reports for the commit ea09a69 to get more accurate results

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #8467   +/-   ##
=========================================
  Coverage     22.20%   22.20%           
  Complexity     7675     7675           
=========================================
  Files           205      205           
  Lines         24178    24178           
  Branches         70       70           
=========================================
  Hits           5369     5369           
  Misses        18741    18741           
  Partials         68       68           
Flag Coverage Δ
autograder 19.22% <ø> (ø)
js 28.47% <ø> (ø)
migrator 99.11% <ø> (ø)
php 20.54% <100.00%> (ø)
python_submitty_utils 71.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar Oct 02 '22 01:10 codecov[bot]

Hi @skara9, thanks for this contribution! Can you maybe fill out the pull request body a little more? In particular, it would be good to add some information about what Latin characters are, why this change is helpful, and why the changes in this PR are the best way to fix the issue in case anyone needs to reference this change in the future.

Although not strictly necessary for this PR to be merged, it might also be good to update the relevant tests to prevent this feature regressing in the future.

williamjallen avatar Oct 02 '22 01:10 williamjallen

Thanks for the reply @williamjallen, I've updated the PR with relevant information and added tests for the change.

skara9 avatar Oct 02 '22 02:10 skara9

Thanks for the bug fix!

MasterOdin avatar Oct 03 '22 15:10 MasterOdin