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

Throw 404 instead of PHP error if email was not found #2634

Merged
merged 1 commit into from
Oct 11, 2016

Conversation

escopecz
Copy link
Sponsor Member

@escopecz escopecz commented Oct 4, 2016

Q A
Bug fix? Y
New feature? N
Related user documentation PR URL /
Related developer documentation PR URL /
Issues addressed (#s or URLs) #2382
BC breaks? N
Deprecations? N

Description:

The linked issue reported a PHP error that was caused by trying to access an email preview on email which did not exist. This PR adds a check if the email exists and throws 404 if not.

Steps to test this PR:

  1. Apply this PR and test again.
  2. You should get 404.

Steps to reproduce the bug:

  1. Try to open some non-existing email preview. For example /email/preview/2499999
  2. You should get Call to a member function isPublished() on null in dev mode or it should be logged if using prod.

@escopecz escopecz added T1 Low difficulty to fix (issue) or test (PR) bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test Hacktoberfest labels Oct 4, 2016
@escopecz escopecz added this to the 2.2.1 milestone Oct 4, 2016
@mqueme
Copy link
Contributor

mqueme commented Oct 11, 2016

like this?

screen shot 2016-10-11 at 11 51 35

@escopecz
Copy link
Sponsor Member Author

@mqueme yes, this is how 404 page looks like in the dev env.

@mqueme
Copy link
Contributor

mqueme commented Oct 11, 2016

then 👍 BTW the 500 error I got before was a different one from your example, but still a 500 error.

@mqueme mqueme added T2 Medium difficulty to fix (issue) or test (PR) pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test and removed ready-to-test PR's that are ready to test labels Oct 11, 2016
@alanhartless
Copy link
Contributor

+1

@alanhartless alanhartless merged commit 53fdade into mautic:staging Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test T1 Low difficulty to fix (issue) or test (PR) T2 Medium difficulty to fix (issue) or test (PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants