Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #222 - "Reset failed jobs" button #235

Closed
wants to merge 2 commits into from

Conversation

@bmcmurray
Copy link

@bmcmurray bmcmurray commented Oct 16, 2012

Abstract out reset job function in jobs.js. Add action bar above and below run table.

in jobs.js. Add action bar above and below run table.
@Krinkle
Krinkle reviewed Oct 16, 2012
View changes
js/job.js Outdated
resetJob($(this).parent('td'));
});

$( document ).on ( 'click' , '.swarm-job-reset-failed', function () {

This comment has been minimized.

@Krinkle

Krinkle Oct 16, 2012
Member

  • (unexpected space in between function name and invocation parentheses)
  • (unexpected space before comma)
@bmcmurray
Copy link
Author

@bmcmurray bmcmurray commented Oct 18, 2012

Cleaned up the code formatting.

if ( $request->getSessionData( "auth" ) === "yes" && $data["jobInfo"]["ownerName"] == $request->getSessionData( "username" ) ) {
$html .= '<script>SWARM.jobInfo = ' . json_encode( $data["jobInfo"] ) . ';</script>';
$action_bar = '<div class="form-actions swarm-item-actions">'
. ' <button class="swarm-job-reset-failed btn btn-info">Reset Failed jobs</button>'

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

Use sentence case ("Reset failed jobs") like the others.

@@ -115,6 +121,10 @@ public static function getUaRunsHtmlRows( $runs, $userAgents ) {
"Open run results for {$userAgents[$uaID]['displaytitle']}"
) . '"></i>'
. '</a>';
$html .=
html_tag_open( 'i', array('class' => 'hover-only swarm-job-reset-single icon-trash pull-right', 'title' => htmlspecialchars(
"Delete results for {$userAgents[$uaID]['displaytitle']}"

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

We don't delete the results, we reset the run in this job. The results have permalinks and will stay available by direct link.

@@ -61,38 +61,49 @@ jQuery(function ( $ ) {
$wipejobErr.hide().text( data.error && data.error.info || 'Action failed.' ).slideDown();
}

function resetJob( $el ) {

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

This resets the run, not the entire job.

}
});
}
};

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

Unexpected semi-colon.

}
}
});
$( document ).on( 'click', '.swarm-job-reset-single', function () {

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

job>run.

@@ -115,6 +121,10 @@ public static function getUaRunsHtmlRows( $runs, $userAgents ) {
"Open run results for {$userAgents[$uaID]['displaytitle']}"
) . '"></i>'
. '</a>';
$html .=
html_tag_open( 'i', array('class' => 'hover-only swarm-job-reset-single icon-trash pull-right', 'title' => htmlspecialchars(

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

This double escapes it. html_tag_open takes care of escaping.

This comment has been minimized.

@Krinkle

Krinkle Nov 2, 2012
Member

This button should only be displayed if the user is authorised for the current project (just like we do with the buttons).

@@ -117,6 +117,14 @@ table.swarm-results tbody td.swarm-status {
font-weight: bold;
}

table.swarm-results tbody td.swarm-status .hover-only {
visibility: hidden;

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

Visibility works fine, but looks a little odd because the text isn't centered anymore and the icons are hidden too.

It also doesn't work well for discoverability (better than double click, which nobody would guess, but hovering to discover isn't ideal either). Lets try opacity.

resetJob( $( this ).parent( 'td' ) );
});

$( document ).on( 'click', '.swarm-job-reset-failed', function () {

This comment has been minimized.

@Krinkle

Krinkle Nov 1, 2012
Member

This one should have a confirmation as well.

@Krinkle Krinkle closed this in ee8dc59 Nov 1, 2012
patrickkettner added a commit to patrickkettner/testswarm that referenced this pull request Nov 5, 2012
Abstract out reset job function in jobs.js.
Add action bar above and below run table.

Add an icon to each cell to allow for an individual runs to be
reset (replacing the old dblclick logic).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.