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

Contact form #1564

Merged
merged 5 commits into from
Sep 14, 2016
Merged

Contact form #1564

merged 5 commits into from
Sep 14, 2016

Conversation

frenchbread
Copy link
Contributor

@frenchbread frenchbread commented Sep 12, 2016

Closes #1572
Closes #884

@frenchbread
Copy link
Contributor Author

Partly fixed, mail settings fetch refactored. Yet, the contact form data is not delivered via email..

@frenchbread
Copy link
Contributor Author

Autoform is already used for the contact form.

@brylie
Copy link
Contributor

brylie commented Sep 13, 2016

@frenchbread note that this is related to the API Feedback form, rather than the Home page contact form.

The API Feedback form is not submitting correctly.

https://github.com/apinf/platform/tree/develop/feedback/client/form

@@ -13,9 +14,12 @@ Meteor.methods({

this.unblock();

// Get email settings
const mailSettings = Settings.findOne().mail;
Copy link
Contributor

@brylie brylie Sep 13, 2016

Choose a reason for hiding this comment

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

This is dangerous code. Settings.findOne() may return undefined, or the .mail attribute may be undefined.

When you use mailSettings.toEmail on line 22, it may throw an error if either of the above cases were true.

@brylie brylie added this to the Sprint 31 milestone Sep 13, 2016
@frenchbread
Copy link
Contributor Author

note that this is related to the API Feedback form, rather than the Home page contact form.

@brylie Right, my bad. Closing this branch

@frenchbread frenchbread deleted the hotfix/feedback-form branch September 13, 2016 12:27
@brylie
Copy link
Contributor

brylie commented Sep 13, 2016

@frenchbread it is OK, and the changes are actually helpful.

Please open an issue related to the 'contact form' and open a PR with this branch.

@brylie brylie restored the hotfix/feedback-form branch September 13, 2016 12:28
@brylie brylie reopened this Sep 13, 2016
@brylie
Copy link
Contributor

brylie commented Sep 13, 2016

I updated the PR description with a new task related to this fix.

@frenchbread please verify whether issue #884 is also fixed by this PR. If so, add 'Closes #884' to the task description. After you have checked #884, add the 'ready for review' label to this PR.

@frenchbread
Copy link
Contributor Author

@brylie Verified. Please review

@frenchbread frenchbread changed the title Hotfix/feedback form Contact orm Sep 13, 2016
@frenchbread frenchbread changed the title Contact orm Contact form Sep 13, 2016
@brylie
Copy link
Contributor

brylie commented Sep 14, 2016

@frenchbread I am merging this now.

Note that we need to add Closes # on a new line for each issue that a PR closes in order for the issue to be properly linked. You can use alternative keywords like 'fixes', etc., but they have to be on a new line for each linked issue.

@brylie brylie merged commit 1986517 into develop Sep 14, 2016
@brylie brylie deleted the hotfix/feedback-form branch September 14, 2016 08:29
@ilarimikkonen ilarimikkonen added this to In Review in apinf/platform Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants