Skip to content

[4.0][com_contact] - fix unpublished contact#30642

Merged
wilsonge merged 1 commit intojoomla:4.0-devfrom
alikon:patch-81
Sep 15, 2020
Merged

[4.0][com_contact] - fix unpublished contact#30642
wilsonge merged 1 commit intojoomla:4.0-devfrom
alikon:patch-81

Conversation

@alikon
Copy link
Contributor

@alikon alikon commented Sep 14, 2020

Pull Request for Issue #30637 .

Summary of Changes

fix

Testing Instructions

see #30637

Actual result BEFORE applying this Pull Request

multiple errors

Expected result AFTER applying this Pull Request

404

@infograf768
Copy link
Member

I have tested this item ✅ successfully on bb6a0ea


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30642.

1 similar comment
@chmst
Copy link
Contributor

chmst commented Sep 14, 2020

I have tested this item ✅ successfully on bb6a0ea


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30642.

@Quy Quy removed the PR-4.0-dev label Sep 14, 2020
@Quy
Copy link
Contributor

Quy commented Sep 14, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30642.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 14, 2020
@Quy Quy added the PR-4.0-dev label Sep 14, 2020
if ($e->getCode() == 404)
{
// Need to go through the error handler to allow Redirect to work.
throw new \Exception($e->getMessage(), 404);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we re-throw the original exception rather than creating a fresh one. At worse we should nest the existing exception as the third param

Copy link
Member

Choose a reason for hiding this comment

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

@wilsonge
That code is the same as in ArticleModel.php. Should the eventual modification be done for both in another PR in order to get that in already for beta4?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure I'm happy. But we definitely should at minimum nest the exceptions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wilsonge did you mean throw $e; ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally yes. Else if that doesn't work for some reason - worse case change this line to

throw new \Exception($e->getMessage(), 404, $e);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ok .... throw $e; works
i'll do a patch with that for both ArticleModel and ContactModel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see #30653

@wilsonge wilsonge merged commit 2d8831b into joomla:4.0-dev Sep 15, 2020
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 15, 2020
@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 15, 2020
@alikon alikon deleted the patch-81 branch September 15, 2020 11:40
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
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.

6 participants