@roland-d @rdeutz @brianteeman when you have some time please take a look on it. Thank you!

1 RuntimeException
Invalid Issue

@brianteeman you need to login

all good

On 2 December 2014 at 12:57, Robert Deutz notifications@github.com wrote:

@brianteeman https://github.com/brianteeman you need to login

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

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

oh wait, I have created a issue on joomla-cms thought there is another repo, don't think it is such a good idea to create issues on our repo just for testing

@rdeutz i think that is the reason for this or i'm wrong?
http://issues.joomla.org/tracker/jtests

@zero-24 yeap you are right

@rdeutz that is why we have jtests repo to for testing ;) BTW I found a small issue because of your test in the CMS repo - tests radio buttons should not be displayed if this is not a Pull Request.

sorry for the confusion, seems there is an outcome by accident :-)

beside this, it is working, thanks for the doing the additions, very useful!

Thanks for testing. Will make a fix (does not affect testing addition) and will be good to merge ;)

@b2z I don't see the test module on the test item on your test website, is that because this item is not a PR? Now I can't see if that section is updated correctly. We can remove the module altogether I think, to force people to write a comment about their test. The only module that should remain is the option for admins to alter test results.

As for the comment one, it works, only style issue is that the successful, unsuccessful part of the string is not colored. See this screenshot for what I mean:

I don't see the test module on the test item on your test website, is that because this item is not a PR?

Yes it is. Also test radio buttons should not be displayed if it is not a PR (will fix it). You should find issue with PR ;)

As for the comment one, it works, only style issue is that the successful, unsuccessful part of the string is not colored.

Yeah I also noticed that. This issue exists in the current code base. Will try to fix it also. BTW this is only when activity is ajax loaded. After page refresh the colors are ok.

We can remove the module altogether I think, to force people to write a comment about their test.

Why not? :) But this should remain:

Tests:
    Successful: 0
    Unsuccessful: 2
    brianteeman, rdeutz

@brianteeman @rdeutz @zero-24 what do you think about removing "Submit test result" button?

@b2z I agree, the test results must remain for sure.

@b2z @roland-d

what do you think about removing "Submit test result" button?

Generaly e.g. if you have a look into a PR on github you can't see the successful or unsuccessful tests and need to access issues.joomla.org. Also if someone mark a unsuccessful test you don't know why it fails.

But If we remove it the users need two posts to reset the test. One post with use the "Not tested" button and after this one post to mark the "successful test".

Sure the JBS Module should be still there and JBS members are able to reset it but if we remove it for normal users there are unable to rest it in a easy way.

@zero-24 you are right. I did not think about the "Not tested" option. May be it is ok to post comment also for "Not tested" because it is not used a lot? And if it is used then joomler, please explain why "Not tested"?

@b2z

And if it is used then joomler, please explain why "Not tested"?

hehe I'm wrong. You need Not tested only if you what to remove your test (successful or unsccessful).

May be it is ok to post comment also for "Not tested" because it is not used a lot?

Yes. This can only happen if someone was added by a JBS Member and or use the false vote radio

@b2z I have not tested jet but do we force the users to enter a test result? Maybe someone only would leave a comment without any testing.

do we force the users to enter a test result

I coded that we do not force.

The model should return null if no test result was submitted.

Then I check it:

{% if (item.userTest is defined and item.userTest == 0) %}
    {% set checked = ' checked="checked"' %}
{% else %}
    {% set checked = '' %}
{% endif %}

When I was testing locally it was working. If you did not submit any test results then the radio buttons should do not be checked. But now I see on the test website that it is not working I will look into it one more time.

So all issues have been solved. @elkuku or @mbabker please review.

And we should decide will we remove tests module or not?

Hey guys! What would be the final decision?

For me we can remove it. As we don't force the user to send a result. But i think @roland-d should / can make a final decision here. Or others from PLT.

I'll look at it tonight.

@zero-24 makes a very valid point that users need 2 posts to reset a test result, this is undesired in my opinion. So let's keep the test module and just add the test results to the bottom of the comment box.

Great. Than it needs only code review ;)

@mbabker Did you get a chance to look at this?

Crap, this is what I forgot LOL

With 2.5 done today, I should be able to get to it tonight, promise :-)

Well @mbabker

OK, I'm mildly lost here. What exactly should I be testing/reviewing? I cloned the live database into my local and checked this branch out, not seeing anything obvious related to logging test results.

Oh, silly me, the item wasn't a pull request

IMO, it looks fine as it is now. No need to drop the full test results module right now, it's a good quick reference spot for that info.

@mbabker if code is ok then good to merge ;)

Thanks @mbabker for review