search

famedly / uia-proxy

GNU Affero General Public License v3.0
0 stars 0 forks source link

fix: Deserialize stored username mappings correctly - [merged] #132

Closed famedly-bot closed 1 year ago

famedly-bot commented 1 year ago

In GitLab by @agraven on Dec 9, 2022, 19:04

Merges agraven/mapping-json-fix -> main

Correctly deserialize the JSON representation of Buffers used when binary persistent IDs are enabled

TODO: File an issue about the bug this MR fixes

TODO: Test if this works as expected with binary PIDs disabled

famedly-bot commented 1 year ago

In GitLab by @agraven on Dec 9, 2022, 19:04

requested review from @kate-shine, @jbecker, @jcgruenhage, @jdreichmann, @ghost, @ghost, @jakibaki, @ghost, and @ghost

famedly-bot commented 1 year ago

In GitLab by @kate-shine on Dec 9, 2022, 19:38

Commented on src/usernamemapper.ts line 41

I'm not well versed in TS, but why can't we keep the IUsernameMapperResult interface and use it here instead of any?

famedly-bot commented 1 year ago

In GitLab by @kate-shine on Dec 9, 2022, 19:38

It's more a language question, It looks like it should work fine ^^

famedly-bot commented 1 year ago

In GitLab by @kate-shine on Dec 9, 2022, 19:38

approved this merge request

famedly-bot commented 1 year ago

In GitLab by @agraven on Dec 10, 2022, 10:57

Commented on src/usernamemapper.ts line 41

The idea with the function is to accept an arbitrary object that we don't know the shape of, and then use type narrowing to check whether it conforms to the expected shape, and throw a TypeError if it doesn't hence the TODO comment about changing it to unknown. If we have an object we already know has the expected shape, the regular constructor function can be used.

Note to self, what i wrote here should be documented in the code

famedly-bot commented 1 year ago

In GitLab by @ghost on Dec 12, 2022, 06:41

mentioned in merge request famedly/company/frontend/famedly-web!805

famedly-bot commented 1 year ago

In GitLab by @jcgruenhage on Dec 12, 2022, 10:56

I've just tried it on dev-beta, where the problem originally surfaced during the dart app migration, and while the migration is still causing trouble, the ball has been passed back to the frontend for now, as the authentication bit now works again. Thanks for the quick fix!

famedly-bot commented 1 year ago

In GitLab by @jdreichmann on Dec 12, 2022, 12:58

approved this merge request