Skip to content

[NEW] make the contact mail addresses a environement variable - #282

Open
maxones25 wants to merge 11 commits into
devfrom
feature/make_contact_mail_address_a_parameter_2
Open

[NEW] make the contact mail addresses a environement variable#282
maxones25 wants to merge 11 commits into
devfrom
feature/make_contact_mail_address_a_parameter_2

Conversation

@maxones25

Copy link
Copy Markdown
Collaborator

Related Issue

Proposed Changes

Additional Info

Checklist

  • Is the code formatted correctly?
  • Unnecessary comments removed?
  • Printout statements removed?
  • If you made backend changes: Did all tests pass? Do you need to adjust some tests or write new ones?
  • If you made frontend changes: Did you test the UI on different devices/ browsers (Firefox, Chrome, Safari, different smartphone sizes)
  • Is your code easy to understand or do you need to insert some comments?
  • Explain the issue for the reviewer and your steps to solve the issue => makes it a lot more easy for the reviewer
  • Label your pull request (frontend/ backend/ testing/ styling/ sql/...)
  • Is the open source message included at the very beginning of every source code file?
  • Are all imports organized, i.e. are the imports up to date and have all package imports been removed?

@maxones25 maxones25 changed the title Feature/make contact mail address a parameter 2 [NEW] make the contact mail addresses a environement variable Nov 11, 2021
@Gabril-E
Gabril-E self-requested a review November 18, 2021 16:36

@MorMundHS-MA MorMundHS-MA left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, the regex change isn't a must-have but the error handling should be improved.

I assume the idea is to allow other schools/organizations use the app more easily. If so we should use proper internationalization to allow customization of the text on the website. https://www.baeldung.com/spring-boot-internationalization

private String domain;
private String tld;

public static MailAddress parse(String mailAddress) throws MailAddressInvalidException {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think its more readable using a regex to split the e-mail.

    private static final Pattern email_splitter_regex = Pattern.compile("(.*)@(.*)\\.(.*)$");

    public static MailAddress parse(String mailAddress) throws InvalidEmailException {
        val match = email_splitter_regex.matcher(mailAddress);
        if(!match.matches()) {
            throw new InvalidEmailException();
        }

        val name = match.group(1);
        val domain = match.group(2);
        val tld = match.group(3);
        return new MailAddress(mailAddress, name, domain, tld);
    }

}

@ResponseStatus(code = HttpStatus.NOT_FOUND, reason = "Mail Address invalid")
public static class MailAddressInvalidException extends RuntimeException {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There already is an InvalidEmailException. But it probably should be more explicit about this being a configuration error vs. a user entering an invalid email. Maybe using a default email like invalid-email@invalid.org + logging an error makes more sense.

import java.util.List;

@AllArgsConstructor @Getter
public class MailAddress {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Am I getting this right, that this class and the whole parsing is only done to decompose the mail address for the mail obfuscation used in the frontend? Wouldn't it be easier to decompose this manually in the properties file and access the elements from the frontend directly? (like so: https://stackoverflow.com/questions/56102116/access-application-properties-value-in-thymeleaf-template)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes, you could also enter the email address already split into the properties. I thought that might be a bit more comfortable in the configuration if the splitting happens automatically.

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.

Make the contact mail addresses a parameter that could be set from outside the source code

3 participants