[#12508] - Only attempt to banner increment impression when needed
- Closed
- 22 Oct 2016
- Medium
- Build: staging
- # 12508
- Diff
- PhilETaylor:banners_fix
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #11969 and other fringe cases
Summary of Changes
This PR is the result of debugging on two live sites with deadly fatal exceptions as per #11969
On code review it was clear this code was low quality, running two loops when one would have done, and also causes issues where there were no items to update, but yet we still tried to execute a query when there were no items to increment impmade to.. this causes a deadly JDatabaseExceptionExecuting, rearranging the code block to only run if needed fixes the exception issue.
Testing Instructions
Find a site where you get the deadly issue reported - apply patch - see its fixed - check that banners impressions are still logged
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Category | ⇒ | Front End Components |
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-10-22 13:45:53 |
| Closed_By | ⇒ | PhilETaylor |
@PhilETaylor i'm just saying this because #11716 was made to solve #11701
Out at the waffle shop with the kids - will re PR when I get home
Sent from my iPhone
On 22 Oct 2016, at 15:15, andrepereiradasilva notifications@github.com wrote:
@PhilETaylor i'm just saying this because #11716 was made to solve #11701
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
@andrepereiradasilva fine, I'll do it the other way, it just doesnt make sense, but with a million banners on one page it will be more performant - yes - however I dont think many people have more than a handful on a page...