Fightpandemics: Unable to ADD new Organization (Show message that email is not unique)

Created on 27 Jun 2020  路  16Comments  路  Source: FightPandemics/FightPandemics

  1. Open"https://staging.fightpandemics.work"
  2. Login with valid email/password
  3. Navigate Profile ->Add Organization
  4. Enter all the details in 'Add Organization' page and click on 'Create Profile' button

Issue# You will observe nothing will happen, user stay on same page, no message or error is displayed.
P.S. attached GIF.

https://share.getcloudapp.com/X6uo8eKy

Difficulty - Medium High Priority MVP - Critical Organization Profile bug

Most helpful comment

@joshmorel @Naraujo13 , seems like for now we could go with what @kylejvh did here https://github.com/FightPandemics/FightPandemics/pull/969 for now right? And implement either 1 or 2 from comment above later?

All 16 comments

The attached gif does not contain your whole process. You might want to use cloudapp to record a small video instead, same process, just allows you to record up to 1 min

And I actually figure out your issue, it looks like you are using the email that is attached to a user. The org email has to be different than the one you have as a user.

And I actually figure out your issue, it looks like you are using the email that is attached to a user. The org email has to be different than the one you have as a user.

https://share.getcloudapp.com/rRulNGKe

@joshmorel I guess we can display a message saying this to the user? so far the user just sees that nothing happens, but the api call just fails cuz there is a user with the same email already.

@rabbi08
image

I guess we can tell the user this?

I guess we can tell the user this?

I created Org Profile with new email ID, and it WORKS with that, so lets display user friendly message, that email already exist.

Adding @vikjoshi

@idjevm @Naraujo13 I'm trying to reuse the alert functionality from user sign up to show an error. I'm wondering why Josue had a different error message. It seems he had the correct validator error message while I get the default. Any idea why? Is it different in production?

Org_Fail

@idjevm @Naraujo13 , can you help @kylejvh with this question here?

I looked at how this was handled in other parts of the app and tried to build out something consistent with what was in place. Let me know of any needed changes, please.

Interesting, it's like this isn't working mongoose-unique-validator isn't working for organization even though I see no difference at all in b/e for one vs the other. This message is coming back from mongodb.

@Naraujo13 is this a hint to why this might be occurring? https://github.com/blakehaswell/mongoose-unique-validator/pull/85/commits/8949fdc57217da287177b5678c695f6ad4bfa70b

We could always roll our own... unique validator.

I was also going to suggest exactly what @kylejvh did as a workaround. I'll review soon.

Interesting, it's like this isn't working mongoose-unique-validator isn't working for organization even though I see no difference at all in b/e for one vs the other. This message is coming back from mongodb.

@Naraujo13 is this a hint to why this might be occurring? blakehaswell/mongoose-unique-validator@8949fdc

We could always roll our own... unique validator.

I was also going to suggest exactly what @kylejvh did as a workaround. I'll review soon.

You're probably right on the hint @joshmorel. We use discriminator to have both IndividualUser and OrganizationUser on the same collection. The unique validator is in the base User model, but we use the specific Organization model at the endpoint, so this is probably the cause for this.
In our case we need to use the more specific model, so we can't switch to the generic one like in the fix you linked.

But not sure if implementing our own is the best idea here, we would have to watch out to not implement it in a way that we do additional queries to check it.
Maybe there is a way to just catch this Mongo exception and change the error message?

Anyway, a single additional query shouldn't be too bad since updating the email won't be frequent.

But not sure if implementing our own is the best idea here, we would have to watch out to not implement it in a way that we do additional queries to check it.

@Naraujo13 I guess my wording was wrong. I don't mean to implement something similar but achieve the same result we were trying to do with mongoose-unique-validator

I think we should either:

1) As you suggest just catch Mongo exception and return better error message. Easiest and somewhat similar to what @kylejvh has already done for this specific issue. Maybe best to begin with. We can change this to a fastify reply decorator used by the pertinent routes.
2) Run a query to check email already exists before user creation or organization create/update. This may be better and not too much hard. Might be better long run because we can also include it before user sign-up.

@joshmorel @Naraujo13 , seems like for now we could go with what @kylejvh did here https://github.com/FightPandemics/FightPandemics/pull/969 for now right? And implement either 1 or 2 from comment above later?

Yeah, I think we can go with this now

I had been meaning to investigate this - https://github.com/FightPandemics/FightPandemics/pull/969#discussion_r449754127 -

But I guess as @idjevm mentioned, it's better then clicking submit and nothing happens. We can implement an improved version after soft launch.

Was this page helpful?
0 / 5 - 0 ratings