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

Use correct logic with custom URL placeholder #162

Closed
1 of 3 tasks
RohanNagar opened this issue May 3, 2018 · 4 comments · Fixed by #176
Closed
1 of 3 tasks

Use correct logic with custom URL placeholder #162

RohanNagar opened this issue May 3, 2018 · 4 comments · Fixed by #176
Assignees
Labels
P1 High priority. Should be addressed by the next release. Size: Medium The work involved in addressing this issue is a sizable amount. 1-2 days of work. Status: Available Work on this issue has not yet started and is available to be worked on. Type: Enhancement An enhancement of an existing feature.
Milestone

Comments

@RohanNagar
Copy link
Owner

This issue relates to a:

  • Bug
  • Suggestion
  • Feature Proposal

Description:

Continuation of #150. The user should be able to (optionally) customize the string that is used as a placeholder for the URL. Here are the following scenarios and what should happen:

  1. No urlPlaceholderString defined and no custom email pages defined

    Use the default CODEGEN-URL and the default HTML/Text files

  2. Custom urlPlaceholderString (not equal to CODEGEN-URL) and no custom email pages

    Log warning and use CODEGEN-URL anyway with the default files

  3. No urlPlaceholderString defined and one or both custom email pages defined

    Use default CODEGEN-URL and the custom pages or one custom page and one default

  4. Custom urlPlaceholderString (not equal to CODEGEN-URL) and one custom email page

    Use the custom placeholder for the one page that is custom, use CODEGEN-URL for the other

  5. Custom urlPlaceholderString (not equal to CODEGEN-URL) and both custom email pages

    Use the custom placeholder for both custom pages

Additional Information:

@RohanNagar RohanNagar added Type: Enhancement An enhancement of an existing feature. P2 Medium priority. Can be addressed within the next few releases. Status: Available Work on this issue has not yet started and is available to be worked on. Size: Medium The work involved in addressing this issue is a sizable amount. 1-2 days of work. labels May 3, 2018
@RohanNagar RohanNagar added this to the v2.0.0 milestone May 3, 2018
@RohanNagar
Copy link
Owner Author

Also want to customize email subject. Want to start using a EmailMessage class with a builder pattern. Also want a separate configuration object within ses for message options, and want configuration to look like:

ses:
  endpoint:
  region:
  fromAddress:
  messageOptions:
    subject:
    bodyHtmlFile:
    bodyTextFile:
    urlPlaceholderString:
    successHtmlFile:

@RohanNagar RohanNagar added Status: In Progress Work on the issue is in progress. and removed Status: Available Work on this issue has not yet started and is available to be worked on. labels May 10, 2018
@RohanNagar
Copy link
Owner Author

RohanNagar commented May 10, 2018

Work started on the email branch

More experimentation on this - taking notes here for me to remember

Create class MessageOptionsConfiguration - holds the config
Create class MessageOptions - holds the values and will be provided by EmailModule based on the config, taking care of defaults and all that
VerificationResource will take in MessageOptions and use that instead of taking in individual strings.

@RohanNagar
Copy link
Owner Author

Thought this was done in #172, but realized that I didn't take care of the special scenarios outlined in the main post. Keeping this open until those are done.

@RohanNagar RohanNagar changed the title Customize Email URL replacement string Use correct logic with custom URL placeholder May 13, 2018
@RohanNagar RohanNagar added P1 High priority. Should be addressed by the next release. Status: Available Work on this issue has not yet started and is available to be worked on. and removed P2 Medium priority. Can be addressed within the next few releases. Status: In Progress Work on the issue is in progress. labels May 13, 2018
@RohanNagar
Copy link
Owner Author

Being worked on in the placeholder branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority. Should be addressed by the next release. Size: Medium The work involved in addressing this issue is a sizable amount. 1-2 days of work. Status: Available Work on this issue has not yet started and is available to be worked on. Type: Enhancement An enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant