Skip to content

Commit e7fb11f

Browse files
mawinter69mawinter69
authored
Directly view existing explanation (#57)
When clicking on the "Explain Error" button when viewing the console output and an explanation already exists a intermediate pseudo dialog was shown to either show the explanation, regenerate or cancel. I think that is not a good UI experience. An existing explanation is now directly shown when clicking in "Explain Error". The card now has 2 buttons in the top right that allow to regenerate and to close the explanation. Even when the explainer is disabled after an explanation has been created for a run, the button will be available. But re-explaining will not be available in that case. The change also prevents that the footer injects anything to a page or loads the js when not on a console url. fixes #52 <!-- Please describe your pull request here. --> After: <img width="1052" height="233" alt="image" src="https://github.com/user-attachments/assets/59c99383-b1c4-4a9f-ae62-9828122db5c6" /> ### Testing done Interactive testing and added unit tests that ensure the div is properly injected. <!-- Comment: Provide a clear description of how this change was tested. At minimum this should include proof that a computer has executed the changed lines. Ideally this should include an automated test or an explanation as to why this change has no tests. Note that automated test coverage is less than complete, so a successful PR build does not necessarily imply that a computer has executed the changed lines. If automated test coverage does not exist for the lines you are changing, you must describe the scenario(s) in which you manually tested the change. For frontend changes, include screenshots of the relevant page(s) before and after the change. For refactoring and code cleanup changes, exercise the code before and after the change and verify the behavior remains the same. --> ### Submitter checklist - [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch! - [x] Ensure that the pull request title represents the desired changelog entry - [x] Please describe what you did - [x] Link to relevant issues in GitHub or Jira - [ ] Link to relevant pull requests, esp. upstream and downstream changes - [ ] Ensure you have provided tests that demonstrate the feature works or the issue is fixed <!-- Put an `x` into the [ ] to show you have filled the information. The template comes from https://github.com/jenkinsci/.github/blob/master/.github/pull_request_template.md You can override it by creating .github/pull_request_template.md in your own repository -->
1 parent cd00de6 commit e7fb11f

File tree

7 files changed

+188
-206
lines changed

7 files changed

+188
-206
lines changed

‎src/main/java/io/jenkins/plugins/explain_error/ConsoleExplainErrorAction.java‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -93,39 +93,6 @@ public void doExplainConsoleError(StaplerRequest2 req, StaplerResponse2 rsp) thr
9393
}
9494
}
9595

96-
/**
97-
* AJAX endpoint to check if an explanation already exists.
98-
* Returns JSON with hasExplanation boolean and timestamp if it exists.
99-
*/
100-
@RequirePOST
101-
public void doCheckExistingExplanation(StaplerRequest2 req, StaplerResponse2 rsp) throws ServletException, IOException {
102-
try {
103-
run.checkPermission(hudson.model.Item.READ);
104-
105-
ErrorExplanationAction existingAction = run.getAction(ErrorExplanationAction.class);
106-
boolean hasExplanation = existingAction != null && existingAction.hasValidExplanation();
107-
108-
rsp.setContentType("application/json");
109-
rsp.setCharacterEncoding("UTF-8");
110-
PrintWriter writer = rsp.getWriter();
111-
112-
if (hasExplanation) {
113-
String response = String.format(
114-
"{\"hasExplanation\": true, \"timestamp\": \"%s\"}",
115-
existingAction.getFormattedTimestamp()
116-
);
117-
writer.write(response);
118-
} else {
119-
writer.write("{\"hasExplanation\": false}");
120-
}
121-
122-
writer.flush();
123-
} catch (Exception e) {
124-
LOGGER.severe("Error checking existing explanation: " + e.getMessage());
125-
rsp.setStatus(500);
126-
}
127-
}
128-
12996
/**
13097
* AJAX endpoint to check build status.
13198
* Returns JSON with buildingStatus to determine if button should be shown. 0 - SUCCESS, 1 - RUNNING, 2 - FINISHED and FAILURE

‎src/main/java/io/jenkins/plugins/explain_error/ConsolePageDecorator.java‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
import hudson.Extension;
44
import hudson.model.PageDecorator;
5+
import hudson.model.Run;
6+
import org.kohsuke.stapler.Ancestor;
7+
import org.kohsuke.stapler.Stapler;
8+
import org.kohsuke.stapler.StaplerRequest2;
59

610
/**
711
* Page decorator to add "Explain Error" functionality to console output pages.
@@ -27,12 +31,30 @@ public boolean isExplainErrorEnabled() {
2731
public String getProviderName() {
2832
return GlobalConfigurationImpl.get().getAiProvider().getProviderName();
2933
}
34+
3035
/**
31-
* Helper method for JavaScript to check if a build is completed.
32-
* Returns true if the plugin is enabled (for JavaScript inclusion),
33-
* actual build status check is done in JavaScript.
36+
* Helper method used by jelly to checked if we're on a console url.
3437
*/
3538
public boolean isPluginActive() {
36-
return isExplainErrorEnabled();
39+
String uri = Stapler.getCurrentRequest2().getRequestURI();
40+
return uri.matches(".*/console(Full)?$");
41+
}
42+
43+
public String getRunUrl() {
44+
Ancestor ancestor = Stapler.getCurrentRequest2().findAncestor(Run.class);
45+
if (ancestor != null && ancestor.getObject() instanceof Run<?, ?> run) {
46+
return run.getUrl();
47+
} else {
48+
return null;
49+
}
50+
}
51+
52+
public ErrorExplanationAction getExistingExplanation() {
53+
Ancestor ancestor = Stapler.getCurrentRequest2().findAncestor(Run.class);
54+
if (ancestor != null && ancestor.getObject() instanceof Run<?, ?> run) {
55+
return run.getAction(ErrorExplanationAction.class);
56+
} else {
57+
return null;
58+
}
3759
}
3860
}

‎src/main/resources/io/jenkins/plugins/explain_error/ConsolePageDecorator/footer.jelly‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,31 @@
11
<?jelly escape-by-default='true'?>
22
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout">
3-
<j:if test="${it.pluginActive}">
3+
<j:set var="active" value="${it.pluginActive}" />
4+
<j:if test="${active}">
5+
<j:set var="enabled" value="${it.explainErrorEnabled}" />
6+
<j:set var="existingExplanation" value="${it.existingExplanation}" />
7+
<j:set var="hasExplanation" value="${existingExplanation != null}" />
8+
<j:if test="${enabled or hasExplanation}">
49
<script src="${rootURL}/plugin/explain-error/js/explain-error-footer.js" type="text/javascript"/>
5-
6-
<div id="explain-error-container" class="jenkins-hidden" data-provider-name="${it.providerName}">
7-
<l:card title="AI Error Explanation">
10+
<j:set var="controls">
11+
<j:if test="${enabled}">
12+
<a href="#" tooltip="${%reexplain(it.providerName)}" class="jenkins-card__reveal eep-generate-new-button">
13+
<l:icon src="symbol-reload" />
14+
</a>
15+
</j:if>
16+
<a href="#" tooltip="${%Close}" class="jenkins-card__reveal eep-close-button">
17+
<l:icon src="symbol-close" />
18+
</a>
19+
</j:set>
20+
<div id="explain-error-container" class="jenkins-hidden" data-run-url="${it.runUrl}" data-provider-name="${it.providerName}"
21+
data-has-explanation="${hasExplanation}" data-plugin-enabled="${enabled}">
22+
<l:card title="AI Error Explanation ${hasExplanation?'(' + existingExplanation.providerName + ')' : ''}" controls="${controls}">
823
<div id="explain-error-spinner" class="jenkins-hidden">
924
<l:spinner text="Analyzing error logs..."/>
1025
</div>
11-
<pre id="explain-error-content" class="jenkins-hidden jenkins-!-margin-bottom-0"></pre>
12-
</l:card>
13-
</div>
14-
15-
<!-- Confirmation Dialog for existing explanation -->
16-
<div id="explain-error-confirm-dialog" class="jenkins-hidden">
17-
<l:card title="AI Error Explanation Exists">
18-
<p>An AI explanation was already generated on <span id="existing-explanation-timestamp"></span>.</p>
19-
<p>Do you want to view the existing explanation or generate a new one?</p>
20-
<div class="jenkins-button-bar">
21-
<button type="button" class="jenkins-button jenkins-button--primary jenkins-!-margin-1 eep-view-existing-button">
22-
View Existing
23-
</button>
24-
<button type="button" class="jenkins-button jenkins-!-margin-1 eep-generate-new-button" tooltip="Provider: ${it.providerName}">
25-
Generate New
26-
</button>
27-
<button type="button" class="jenkins-button jenkins-!-margin-1 eep-cancel-button">
28-
Cancel
29-
</button>
30-
</div>
26+
<pre id="explain-error-content" class="jenkins-!-margin-bottom-0 ${hasExplanation?'':'jenkins-hidden'}">${hasExplanation ? existingExplanation.explanation : ''}</pre>
3127
</l:card>
3228
</div>
29+
</j:if>
3330
</j:if>
3431
</j:jelly>

‎src/main/resources/io/jenkins/plugins/explain_error/ConsolePageDecorator/footer.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
reexplain=Re-explain with {0}

‎src/main/webapp/js/explain-error-footer.js‎

Lines changed: 38 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
document.addEventListener('DOMContentLoaded', function () {
2-
if (
3-
window.location.pathname.match(/\/console(Full)?$/) &&
4-
!window.location.pathname.includes('/error-explanation')
5-
) {
6-
checkBuildStatusAndAddButton();
7-
}
2+
checkBuildStatusAndAddButton();
83
// Moved from the second DOMContentLoaded listener
94
const container = document.getElementById('explain-error-container');
105
const consoleOutput =
@@ -14,12 +9,6 @@ document.addEventListener('DOMContentLoaded', function () {
149
if (container && consoleOutput && consoleOutput.parentNode) {
1510
consoleOutput.parentNode.insertBefore(container, consoleOutput);
1611
}
17-
18-
// Add the confirmation dialog to the page
19-
const dialogContainer = document.getElementById('explain-error-confirm-dialog');
20-
if (dialogContainer && consoleOutput && consoleOutput.parentNode) {
21-
consoleOutput.parentNode.insertBefore(dialogContainer, consoleOutput);
22-
}
2312
});
2413

2514
function checkBuildStatusAndAddButton() {
@@ -36,8 +25,10 @@ function checkBuildStatusAndAddButton() {
3625
}
3726

3827
function checkBuildStatus(callback) {
39-
const basePath = window.location.pathname.replace(/\/console(Full)?$/, '');
40-
const url = basePath + '/console-explain-error/checkBuildStatus';
28+
const container = document.getElementById('explain-error-container');
29+
const basePath = container.dataset.runUrl
30+
const rootURL = document.head.getAttribute("data-rooturl");
31+
const url = rootURL + '/' + basePath + 'console-explain-error/checkBuildStatus';
4132

4233
const headers = crumb.wrap({
4334
"Content-Type": "application/x-www-form-urlencoded",
@@ -65,15 +56,6 @@ function addExplainErrorButton() {
6556
return;
6657
}
6758

68-
// First try to find the existing console button bar
69-
const consoleButtonBar =
70-
document.querySelector('.console-actions') ||
71-
document.querySelector('.console-output-actions') ||
72-
document.querySelector('.console-controls') ||
73-
document.querySelector('[class*="console"][class*="button"]') ||
74-
document.querySelector('#console .btn-group') ||
75-
document.querySelector('.jenkins-button-bar');
76-
7759
// Try to find buttons by their text content
7860
let buttonContainer = null;
7961
const downloadButtons = Array.from(document.querySelectorAll('a, button')).filter(el =>
@@ -98,8 +80,7 @@ function addExplainErrorButton() {
9880
console.warn('Console output element not found');
9981
setTimeout(function() {
10082
// Only retry if the button doesn't exist yet and we're still on a console page
101-
if (!document.querySelector('.explain-error-btn') &&
102-
window.location.pathname.match(/\/console(Full)?$/)) {
83+
if (!document.querySelector('.explain-error-btn')) {
10384
checkBuildStatusAndAddButton();
10485
}
10586
}, 3000);
@@ -108,120 +89,80 @@ function addExplainErrorButton() {
10889

10990
const container = document.getElementById('explain-error-container');
11091
const providerName = container.dataset.providerName;
111-
112-
const explainBtn = createButton('Explain Error', 'jenkins-button explain-error-btn', explainConsoleError, providerName);
92+
const hasExplanation = container.dataset.hasExplanation === 'true';
93+
const enabled = container.dataset.pluginEnabled === 'true';
94+
if (!enabled && !hasExplanation) {
95+
return;
96+
}
97+
const buttonText = 'Explain Error';
98+
const callback = hasExplanation ? showExistingExplanation : explainConsoleError;
99+
const explainBtn = createButton(buttonText, 'jenkins-button explain-error-btn', callback, providerName);
113100

114101
// If we found the button container, add our button there
115102
if (buttonContainer) {
116103
buttonContainer.insertBefore(explainBtn, buttonContainer.firstChild);
117104
Behaviour.applySubtree(buttonContainer, true);
118-
119-
} else if (consoleButtonBar) {
120-
consoleButtonBar.appendChild(explainBtn);
121105
} else {
122106
// Fallback: create a simple container above console output
123107
const container = document.createElement('div');
124108
container.className = 'explain-error-container';
125109
container.style.marginBottom = '10px';
126110
container.appendChild(explainBtn);
127111
consoleOutput.parentNode.insertBefore(container, consoleOutput);
112+
Behaviour.applySubtree(container, true);
128113
}
129114
}
130115

116+
function showExistingExplanation() {
117+
const container = document.getElementById('explain-error-container');
118+
container.classList.remove('jenkins-hidden');
119+
}
120+
131121
function createButton(text, className, onClick, providerName) {
132122
const btn = document.createElement('button');
133123
btn.textContent = text;
134124
btn.className = className;
135-
btn.onclick = onClick;
125+
btn.onclick = function() {
126+
onClick(false);
127+
};
136128
btn.setAttribute("tooltip", "Provider: " + providerName);
137129
return btn;
138130
}
139131

140132
function explainConsoleError() {
141133
// First, check if an explanation already exists
142-
checkExistingExplanation();
143-
}
144-
145-
function checkExistingExplanation() {
146-
const basePath = window.location.pathname.replace(/\/console(Full)?$/, '');
147-
const url = basePath + '/console-explain-error/checkExistingExplanation';
148-
149-
const headers = crumb.wrap({
150-
"Content-Type": "application/x-www-form-urlencoded",
151-
});
152-
153-
fetch(url, {
154-
method: "POST",
155-
headers: headers,
156-
body: ""
157-
})
158-
.then(response => response.json())
159-
.then(data => {
160-
if (data.hasExplanation) {
161-
// Show confirmation dialog
162-
showConfirmationDialog(data.timestamp);
163-
} else {
164-
// No existing explanation, proceed with new request
165-
sendExplainRequest(false);
166-
}
167-
})
168-
.catch(error => {
169-
console.warn('Error checking existing explanation:', error);
170-
// If check fails, proceed with new request
171134
sendExplainRequest(false);
172-
});
173135
}
174136

175-
function showConfirmationDialog(timestamp) {
176-
const dialog = document.getElementById('explain-error-confirm-dialog');
177-
const timestampSpan = document.getElementById('existing-explanation-timestamp');
178-
179-
if (timestampSpan) {
180-
timestampSpan.textContent = timestamp;
181-
}
182-
183-
dialog.classList.remove('jenkins-hidden');
184-
185-
// Hide other elements
186-
hideContainer();
187-
}
188137

189-
Behaviour.specify(".eep-view-existing-button", "ExplainErrorView", 0, function(e) {
190-
e.onclick = viewExistingExplanation;
138+
Behaviour.specify(".eep-generate-new-button", "ExplainErrorView", 0, function(e) {
139+
e.onclick = function(event) {
140+
event.preventDefault();
141+
generateNewExplanation();
142+
};
191143
});
192144

193-
194-
function viewExistingExplanation() {
195-
hideConfirmationDialog();
196-
sendExplainRequest(false); // This will return the cached result
197-
}
198-
199-
Behaviour.specify(".eep-generate-new-button", "ExplainErrorView", 0, function(e) {
200-
e.onclick = generateNewExplanation;
145+
Behaviour.specify(".eep-close-button", "ExplainErrorView", 0, function(e) {
146+
e.onclick = function(event) {
147+
event.preventDefault();
148+
hideContainer();
149+
};
201150
});
202151

203152
function generateNewExplanation() {
204-
hideConfirmationDialog();
205153
clearExplanationContent();
206154
sendExplainRequest(true); // Force new explanation
207155
}
208156

209-
Behaviour.specify(".eep-cancel-button", "ExplainErrorView", 0, function(e) {
210-
e.onclick = cancelExplanation;
211-
});
212-
213157
function cancelExplanation() {
214158
hideConfirmationDialog();
215159
}
216160

217-
function hideConfirmationDialog() {
218-
const dialog = document.getElementById('explain-error-confirm-dialog');
219-
dialog.classList.add('jenkins-hidden');
220-
}
221-
222161
function sendExplainRequest(forceNew = false) {
223-
const basePath = window.location.pathname.replace(/\/console(Full)?$/, '');
224-
const url = basePath + '/console-explain-error/explainConsoleError';
162+
const container = document.getElementById('explain-error-container');
163+
const basePath = container.dataset.runUrl
164+
const rootURL = document.head.getAttribute("data-rooturl");
165+
const url = rootURL + '/' + basePath + 'console-explain-error/explainConsoleError';
225166

226167
const headers = crumb.wrap({
227168
"Content-Type": "application/x-www-form-urlencoded",
@@ -292,7 +233,6 @@ function hideContainer() {
292233

293234
function clearExplanationContent() {
294235
const content = document.getElementById('explain-error-content');
295-
if (content) {
236+
content.classList.add('jenkins-hidden');
296237
content.textContent = '';
297-
}
298238
}

0 commit comments

Comments
 (0)