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

Send test email UI bugs with port #3010

Closed
nebulade opened this issue Mar 8, 2024 · 6 comments
Closed

Send test email UI bugs with port #3010

nebulade opened this issue Mar 8, 2024 · 6 comments

Comments

@nebulade
Copy link

nebulade commented Mar 8, 2024

Describe the bug
When an admin changes the email smtpSecurity dropdown. The port gets automatically reset to the usual default ports. However the issue is that the port now becomes a type string when sent further down to test the mail sending.

The test mail rest call would send this JSON:

{"server":"mail","port":"465","auth":true,"security":"SSL","use....

The REST api error handled at https://github.com/espocrm/espocrm/blob/master/application/Espo/Tools/Email/Api/PostSendTest.php#L91 gets triggered then. (This is correct)

Now clearing the port field and manually typing that value, the field correctly becomes a number. The input type is actually set to text rather than probably the more correct number

Furthermore if 4 digit ports are put in, it would render with a . or , depending on the locale: 2,525

Screenshots
just for context where this is in the UI:
image

EspoCRM version
8.1.5

Additional context
https://forum.cloudron.io/topic/10522/outbound-email-settings-being-overwritten-when-app-is-restarted/22

@yurikuzn
Copy link
Contributor

yurikuzn commented Mar 8, 2024

image

I could not reproduce.

Bug report w/o steps to reproduce is my woe.

@yurikuzn
Copy link
Contributor

yurikuzn commented Mar 8, 2024

Maybe your instance is broken. I assume JS error occurred before fields fully initiated. I tested multiple scenarios (had to, as absence of steps to reproduce made to try multiple ones), on two instances. Integer is sent.

Not translated field labels on the screenshot hints that something is wrong with your instance.

@yurikuzn yurikuzn closed this as not planned Won't fix, can't repro, duplicate, stale Mar 8, 2024
@nebulade
Copy link
Author

nebulade commented Mar 9, 2024

Ah sorry for not writing them in a list, I guess I thought the bug description outlines the steps already. Will reformat next time.

There are no missing JS files or other Javascript errors reported in the browser console though. I tried to also adjust bandwidth to test if this might be a race or so, but also that didn't reveal anything further.

So that text input field should be of type text then and some other js code would do the formatting? Asking since https://github.com/espocrm/espocrm/blob/master/client/src/views/admin/outbound-emails.js#L131 appears to also set a string.

@yurikuzn
Copy link
Contributor

yurikuzn commented Mar 9, 2024

That's why steps to reproduce are so important in bug reports for big apps. We have 3 different places where SMTP is configured. I tried two and forgot about existence of the 3rd. I'll look into.

@yurikuzn yurikuzn reopened this Mar 9, 2024
@yurikuzn
Copy link
Contributor

yurikuzn commented Mar 9, 2024

Fix: 0832faa. Thanks for help.

BTW, it's possible to use Group Email Account as a system account. SMTP parameters in settings were considered by me as a legacy, that's why I forgot about them. In future it's likely that these settings will be forcibly migrated to a Group Email Account.

@yurikuzn yurikuzn closed this as completed Mar 9, 2024
@nebulade
Copy link
Author

Thanks for the quick fix and also the note about the Group Email Account, we will rework our app package startup script then to use that forward.

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

No branches or pull requests

2 participants