For me it is not the best approach - to generate additional SQL query (and this is not a simple SELECT) in addition to the list of issues.

I contemplated grabbing a random from the list returned previously but there is overhead with doing additional PHP there as well (not to mention that will only get a first page result and based on the user's current filtering status).

Speed Test Results
Under 0.5ms response time for the query if we do not have the comment count included.

~ 3ms response time with comment count

Why not to make it as I've suggested? Just run it when it's requested ;)

Maybe make up the URL and a proper controller first:
"tracker/:project_alias/random" : "\\Tracker\\Controller\\Issue\\RandomController", or something, then do the work here. You may even use those links in forum posts etc.
Sure we don't want any additional queries, if possible ;)

Also I'm not sure about the number 5 for activities. Maybe we should filter only for open issues.
If you want to reduce it to 5 comments you should also check the event column for comment

Let me switch it to link with a call as suggested @elkuku and run it on click. I'm not sold on the comment part either at the moment. I like the idea.

This looks much better now
@b2z any further concerns ?

That what I was talking about ;) @dbhurley gj

@dbhurley please update it to get it in ;)

@dbhurley please update one more time - there were minor fixes in the master and I cannot test without them :)

Still shows as mergeable for me.

BTW for those who does not know about interesting feature Checking out Pull Requests locally. That allows to test PRs.

@dbhurley I cannot test it - fatal error fix not in there...

ah, ok, I'll take a look and see what the latest updates are.

Just merge the master in ;)

Fatal error: Call to undefined method App\Tracker\Controller\Issue\Random::getApplication() in /vagrant/src/App/Tracker/Controller/Issue/Random.php on line 49

You still need to update your code. Application now is in the container. Model should be instantiated with the db driver (you can get it from the container also).

:) hadn't got there yet, was just merging and pushing and then will update controller. I'll be done in a second, wrapping up what I'm working on now...

Ok np ;) Waiting for test

So I updated my code but I think I might see a problem on the IssueModel with the container? This should be testable now though.

Fatal error: Class 'App\Tracker\Model\Container' not found in /vagrant/src/App/Tracker/Model/IssueModel.php on line 160. Why do you need getProject() method in the model?

I didn't add it I didn't think, my fault if I did. I don't need it. :p

Thanks,
David Hurley
On Jan 15, 2014 3:09 PM, "Dmitry Rekun" notifications@github.com wrote:

Fatal error: Class 'App\Tracker\Model\Container' not found in
/vagrant/src/App/Tracker/Model/IssueModel.php on line 160. Why do you need
getProject() method in the model?

—
Reply to this email directly or view it on GitHub#237 (comment)
.

Hehe :) Well without it code works Just remove this method and fix PHPCS issue in getRandomItem() method and it will be ready to merge

Cmon @dbhurley I know you can do it

um...done I thought :) - same day!

Nope :)

Unfortunately our testing method have changed since this PR was opened and are now failing - please correct

From where there are language files failing?

There are also errors in language files, but these will not cause the Travis test to fail (yet) - but checkstyle errors do

Ah, ok. So these errors in language files existed earlier.

Currently the random functionality shows also closed issues and sometimes I get "invalid issues"

Hmm, but how it could be?

->where($this->db->quoteName('s.closed') . '=' . 0)

I hope it's not a PEBCAK again, but I get all sorts of unwanted results on http://issues.joomla.org/tracker/joomla-cms/random
Can you confirm this?

I confirm. Interesting, because from the first look the query is ok.

I believe it should return the issue number instead of the id, as this is used in the url