[#9976] - [com_banners] clients view: Banners counting
- Fixed in Code Base
- 2 May 2016
- Medium
- Build: staging
- # 9976
- Diff
- andrepereiradasilva:com_banners_clients_grouping
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Pull Request for Improvement.
Summary of Changes
This PR adds the counting columns like the ones that exist in categories and menus to the com_banners clients view.
Also does some minor changes in code.
Before PR
After PR
Testing Instructions
- Use latest staging and apply this patch
- Go to Extensions -> Banners -> Clients and create some banner clients
- Go to Extensions -> Banners -> Banners, create some banners and associate then to the clients created in 2.
- Go again to Extensions -> Banners -> Clients and test the counting and their links.
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
?
|
||
| Title |
|
||||||
| Category | ⇒ | Components UI/UX |
| Labels | |||
| Category | Components UI/UX | ⇒ | Components Language & Strings UI/UX |
| Labels | |||
I have tested this item 
unsuccessfully on 31ad9c3
I tested with testing data and found following bug:
Because each client only had 1 banner, I copied one banner and saved it with a new name.
Result: One unpublished and one published counted for this client => OK.
Then I published the copied banner.
Result: Still one published only, and zero unpublished => Wrong for published.
Reason: You count the distinct number of client ids, but you have to count the banners with the particular client IDs.
Means you have to change in your query in line 225 the "->select('cid, COUNT(DISTINCT cid) AS count_published')" to "->select('cid, COUNT(cid) AS count_published')", i.e. just remove the "DISTINCT".
Since I am an Oracle SQL expert on guru level you can trust me.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
@andrepereiradasilva See andrepereiradasilva#32
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
| Labels | |||
This PR has received new commits.
CC: @brianteeman, @richard67
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
@brianteeman How did you test this? With 1 banner of each status, so the count always would correctly show 1?
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
ok @richard67 thanks for the hint. solved the question. please retest
yes - retesting it now
On 19 April 2016 at 10:39, Richard Fath notifications@github.com wrote:
@brianteeman https://github.com/brianteeman How did you test this? With
1 banner of each status, so the count always would correctly show 1?
This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/9976
https://issues.joomla.org/tracker/joomla-cms/9976.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9976 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
ups sorry there seems also be a problem with archived. wait a sec
This PR has received new commits.
CC: @brianteeman, @richard67
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
ok archived solved too.
@andrepereiradasilva If you find time, please correct the code comments, too: They contain "menu" where it should be "banners", e.g.
// Get the unpublished menu counts.
I assume this could be also the case for your other recent PRs?
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
Or
// Get the menu types of menus in the list.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
That was a copy paste problem. will correct. thanks
no, this is the only PR that use counts.
The other ones are just for adding the max level filters.
Ah yes sorry, was confused it seems. Sure, only this one. I will test anyway because is only a comments problem.
This PR has received new commits.
CC: @brianteeman, @richard67
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
ok, comments also corrected. sorry for that guys.
please retest.
I have tested this item 
successfully on a4501d6
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
I have tested this item successfully on a4501d6
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.
@richard67 @brianteeman thanks for testing!
| Status | Pending | ⇒ | Ready to Commit |
| Labels | |||
| Labels |
Added:
?
|
||
| Milestone |
Added: |
||
technically this is a new feature and it doesn't fix a bug so it has to go into 3.6.0
| Milestone |
Added: |
||
| Milestone |
Removed: |
||
It fixes a bug in that the counting is present elsewhere but not in this
component ;)
IMHO sometimes a "new feature" is created to solve a logical inconsistency (bug).
But, of course, i leave to you maintainers to decide that.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-02 06:14:34 |
| Closed_By | ⇒ | rdeutz |
| Labels |
Removed:
?
|
||
I have tested this item
successfully on 31ad9c3
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9976.