Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upodate dotnet SDK to allow null value in user model #800

Conversation

theemaster
Copy link
Contributor

@theemaster theemaster commented Mar 13, 2024

What does this PR do?

Fix User Model to handle null values for password, hash and hashOptions.

Fixes appwrite/sdk-for-dotnet#30

Test Plan

When using a JWT to authenticate and use Account.Get() there should no longer be an error.

Related PRs and Issues

Credit

Credit goes to @joaquingrech.

@nove1398
Copy link

I am experiencing this same error, was debugging a bit and couldn't find why on my side

@joaquingrech
Copy link

I am experiencing this same error, was debugging a bit and couldn't find why on my side

It is because they never merged the fix. It is also in the latest 1.5. I'm surprised it passed release since if you don't use a password and login using Google or any other auth system, all queries for that user will always fail.

@theemaster
Copy link
Contributor Author

theemaster commented Apr 14, 2024

Because the fix hasn't been reviewed yet and merged/released I have forked the dotnet repository and created a temporary nuget package with the package name theemaster.appwrite. You could use it temporarily until the fix has been released.

@nove1398
Copy link

nove1398 commented Apr 14, 2024

How can we get some more eyes on this issue? The changes seem small enough where it could get applied as a patch.

@nove1398
Copy link

I can confirm for me the password key error stops with your changes, but now i am getting it for mfa as well lol, i think the dictionary values should all be checked by TryGet in this User class

Copy link
Member

@adityaoberai adityaoberai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that these are now nullable @lohanidamodar
image

LGTM 👍

@adityaoberai
Copy link
Member

I can confirm for me the password key error stops with your changes, but now i am getting it for mfa as well lol, i think the dictionary values should all be checked by TryGet in this User class

Only the Mfa field or Totp also?

@nove1398
Copy link

I can confirm for me the password key error stops with your changes, but now i am getting it for mfa as well lol, i think the dictionary values should all be checked by TryGet in this User class

Only the Mfa field or Totp also?

Totp too, however I know what my problem is, I was using cloud with the fix meaning the response is trying to parse data the Server is not providing. When I use an older version of the fix it works as expected, and I am able to continue my dev work. Thanks for addressing this so quickly

@adityaoberai
Copy link
Member

I can confirm for me the password key error stops with your changes, but now i am getting it for mfa as well lol, i think the dictionary values should all be checked by TryGet in this User class

Only the Mfa field or Totp also?

Totp too, however I know what my problem is, I was using cloud with the fix meaning the response is trying to parse data the Server is not providing. When I use an older version of the fix it works as expected, and I am able to continue my dev work. Thanks for addressing this so quickly

Ah yes. That would work fine as soon as cloud is upgraded to 1.5.*

@lohanidamodar, in that case we should be good to merge this

@nove1398
Copy link

nove1398 commented Apr 15, 2024

I can confirm for me the password key error stops with your changes, but now i am getting it for mfa as well lol, i think the dictionary values should all be checked by TryGet in this User class

Only the Mfa field or Totp also?

Totp too, however I know what my problem is, I was using cloud with the fix meaning the response is trying to parse data the Server is not providing. When I use an older version of the fix it works as expected, and I am able to continue my dev work. Thanks for addressing this so quickly

Ah yes. That would work fine as soon as cloud is upgraded to 1.5.*

@lohanidamodar, in that case we should be good to merge this

Thanks #TeamAppwrite ! I am happy to know .Net is not the forgotten language lol :) <3 I look forward to the cloud migration to 1.5 in the coming weeks. I have been working with some clients and ultimately I am recommending them to come onboard to the Appwrite platform. #DevRelations

@abnegate abnegate merged commit 6f7a40b into appwrite:master May 3, 2024
1 check passed
@stnguyen90 stnguyen90 changed the title Fix 30 add null value handling in user model Upodate dotnet SDK to allow null value in user model May 15, 2024
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.

🐛 Bug Report: Bug acquiring users on server side
5 participants