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

836: Send verification emails #870

Conversation

sarahsporck
Copy link
Contributor

If one email cannot be sent (, e.g. because of invalid from address) then the application is not persisted.

Copy link
Member

@michael-markl michael-markl left a comment

Choose a reason for hiding this comment

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

If one email cannot be sent (, e.g. because of invalid from address) then the application is not persisted.

Hmm... Not sure if this is a good approach. I think it would be a good approach, if the user got a proper error message when trying to submit the application. Without a proper error message, the user would have to guess what the problem is...

@sarahsporck
Copy link
Contributor Author

If one email cannot be sent (, e.g. because of invalid from address) then the application is not persisted.

Hmm... Not sure if this is a good approach. I think it would be a good approach, if the user got a proper error message when trying to submit the application. Without a proper error message, the user would have to guess what the problem is...

I think we generally are really wonky with our error handling. Imo the graphql interface should always return sth like .

{
    "status": <http_status_like>,
    "message": <maybe_optional_because_of_translations>,
    "result": <only_for_queries>
}

I'll open an issue for this.
In this case though I think it is often logical to fail here. But you are right. A better error message should be displayed, although I'm unsure what to return in an error case.

@sarahsporck sarahsporck force-pushed the 836-applicationverification-send-verification-e-mails-after-creating-an-application branch from 1f7c2f8 to 42a7140 Compare March 17, 2023 16:52
@michael-markl
Copy link
Member

I think we generally are really wonky with our error handling. Imo the graphql interface should always return sth like .

We are super wonky, we never introduced a concept for handling errors^^

Related: #324

The alternative to your suggestion is to use the graphql philosophy and use/improve GraphQL errors: https://www.apollographql.com/docs/react/data/error-handling/

@sarahsporck
Copy link
Contributor Author

If one email cannot be sent (, e.g. because of invalid from address) then the application is not persisted.

After thinking about this a while I think this is pretty difficult to handle. consider two emails already have been sent and one failed. In that case the organization mitarbeiter will still receive emails with an (invalid) link then, which feels much like spam to me. Therefore I reconsidered that we do not roll back here and let the mitarbeiter of the region handle the situation which would go like this in the optimal situation.
An application with three organisations has been created. And sending the mail to two of the organisations has worked fine. However, one failed. The two organisations will verify the application successfully, but the one who has not received the email address has not. In that case the mitarbeiter of the entitlementcard will contact the mitarbeiter at the organization him:herself after maybe 2 weeks.
Showing an error to the applicant in this case has no benefit I think, as he:she cannot interfere or improve the situation. I think the best solution regarding "usability" would be to set the verification in an error state in the db, to let the mitarbeiter of the entitlementcard know he:she has to interfere.

@sarahsporck sarahsporck force-pushed the 836-applicationverification-send-verification-e-mails-after-creating-an-application branch from 906efa9 to b11a414 Compare March 20, 2023 13:20
@sarahsporck sarahsporck merged commit fdac2cf into main Mar 20, 2023
@sarahsporck sarahsporck deleted the 836-applicationverification-send-verification-e-mails-after-creating-an-application branch March 20, 2023 13:46
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.

[ApplicationVerification] Send verification E-Mails after creating an application
2 participants