Skip to content

[feature] 4-allow-sending-the-submissions-to-multi-emails #8

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 9, 2019
Merged

[feature] 4-allow-sending-the-submissions-to-multi-emails #8

merged 4 commits into from
Oct 9, 2019

Conversation

@Alejando
Copy link
Contributor

@Alejando Alejando commented Oct 8, 2019 •
edited
Loading

Changes

  • allow adding multi emails in the target email field
  • add missing translations

Ticket

Evidence

Screen Recording 2019-10-08 at 04 47 PM

@Alejando Alejando changed the base branch from master to development October 8, 2019 21:09
@Alejando Alejando force-pushed the feature/4-allow-sending-the-submissions-to-multi-emails branch from 6eb3a53 to 4d50899 Compare October 8, 2019 21:34
YanCarlos
YanCarlos suggested changes Oct 8, 2019
View reviewed changes
spec/models/dynamic_forms/custom_form_spec.rb Outdated
end

context 'custom validations' do
let(:custom_form) { build(:custom_form, target_email: '[email protected] example') }
Copy link
Contributor

@YanCarlos YanCarlos Oct 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think you need to test the happy and unhappy path and could you add another case for different items separator? please

something like | / :

@Alejando Alejando force-pushed the feature/4-allow-sending-the-submissions-to-multi-emails branch 2 times, most recently from 6496c66 to a7ef1f4 Compare October 8, 2019 23:16
YanCarlos
YanCarlos approved these changes Oct 8, 2019
View reviewed changes
Copy link
Contributor

@YanCarlos YanCarlos left a comment

Choose a reason for hiding this comment

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

LGTM

@Alejando Alejando force-pushed the feature/4-allow-sending-the-submissions-to-multi-emails branch from a7ef1f4 to 5185cf6 Compare October 9, 2019 23:06
@Alejando Alejando merged commit aa95209 into development Oct 9, 2019
@Alejando Alejando deleted the feature/4-allow-sending-the-submissions-to-multi-emails branch October 9, 2019 23:08
Alejando added a commit that referenced this pull request Oct 9, 2019
@Alejando
[feature] 4-allow-sending-the-submissions-to-multi-emails (#8)
e130605
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@YanCarlos YanCarlos YanCarlos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Alejando @YanCarlos