Skip to content

Conversation

@rra
Copy link
Member

@rra rra commented Jul 9, 2021

We were passing the user's name in an HTTP header, but HTTP headers
only truly support ASCII (and, if not ASCII, use ISO 8859-1). This
means we cannot represent many people's names.

Rather than use a more complicated encoding solution to pass the
name down, change SUIT to use the same behavior as squareone and only
show the user's username. Do this by not setting the first or last
name attributes in UserInfo, which should trigger the Firefly
JavaScript layer to fall back to displaying only the username.

We were passing the user's name in an HTTP header, but HTTP headers
only truly support ASCII (and, if not ASCII, use ISO 8859-1).  This
means we cannot represent many people's names.

Rather than use a more complicated encoding solution to pass the
name down, change SUIT to use the same behavior as squareone and only
show the user's username.  Do this by not setting the first or last
name attributes in UserInfo, which should trigger the Firefly
JavaScript layer to fall back to displaying only the username.
@rra rra requested a review from gpdf July 9, 2021 16:53
@rra
Copy link
Member Author

rra commented Jul 9, 2021

Alas, this doesn't seem to work the way that I thought it would from the JavaScript code. I thought this piece of code:

https://github.com/Caltech-IPAC/firefly/blob/84ed84538d5b84a4b5d915a3388edf6a59d22f2e/src/firefly/js/ui/Banner.jsx#L78

would fall back to username, but alas it seems to produce an empty string. I'm not sure why given that first name and last name should both be empty strings. (I tested this by changing Gafaelfawr to not pass X-Auth-Request-Name, which should trigger the existing code to return both as empty strings. This is subtlely different than what this PR would do, which is return null for both, but it looked like it should trigger the same JavaScript behavior.)

This may require someone with more knowledge of the Firefly code to take a quick glance and tell me what I'm missing.

@gpdf
Copy link
Contributor

gpdf commented Jul 9, 2021

We'll take a look at this, thanks.

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.

3 participants