'nuff said.

CommentFileSizeAuthor
#2 pift.hide_ops_if_no_testid-1298720-2.patch839 bytesjthorson

Comments

boombatower’s picture

boombatower commented

indeed

jthorson’s picture

jthorson commented
Status: Active » Needs review
StatusFileSize
new pift.hide_ops_if_no_testid-1298720-2.patch839 bytes

Patch.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs review » Needs work 
+++ b/pift.pages.inc
@@ -195,10 +195,10 @@ function theme_pift_attachments(array $files, $closed) {
+    if (($file['status'] > PIFT_STATUS_QUEUE || $file['message']) && !empty($file['test_id'])) {

Do we need the test on $file['message'] here? If so, shouldn't it be !empty($file['message']? But I suspect we don't need it.

I'm good with this.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Needs work » Needs review

Didn't mean to mark CNW.

jthorson’s picture

jthorson commented

The $file['message'] was part of the original code ... as I wasn't 100% sure what removing it might affect, I simply left it there.

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

I'd be in favor of fixing things when we touch them. If we can't figure out what that is, then remove it. If we *can* figure out what it is, change it to !empty()

jthorson’s picture

jthorson commented

I'm guessing it covers the re-test case, where the test could currently be queued, but a previous result is present in $file['message'] ... in this case, the 'view test' link can is still valid even when the test is queued, since the original test result already exists on qa.d.o.

jthorson’s picture

jthorson commented

Committed to 6.x-2.x (930b800)

jthorson’s picture

jthorson commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.