Skip to content

Commit f96e8b0

Browse files
authored
fix: "Mark as Final" unexpectedly disabled (#1650)
* Add missing testResultsLength to TEST_QUEUE_EXPANDED_ROW_QUERY * Update snapshots * Update mocks * Ignore addViewerResolver error if not critical * Fix not being able to close mark as final dialog
1 parent 17e1141 commit f96e8b0

File tree

7 files changed

+83
-19
lines changed

7 files changed

+83
-19
lines changed

client/components/TestQueue/Actions.jsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ const Actions = ({
175175
content={content}
176176
closeLabel="Cancel"
177177
staticBackdrop={true}
178+
useOnHide={true}
179+
handleClose={hideConfirmationModal}
178180
actions={[
179181
{
180182
label: 'Confirm',

client/components/TestQueue/queries.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ export const TEST_QUEUE_EXPANDED_ROW_QUERY = gql`
278278
}
279279
draftTestPlanRuns {
280280
...TestPlanRunFields
281+
testResultsLength
281282
}
282283
}
283284
}

client/tests/__mocks__/GraphQLMocks/TestQueuePageBaseMock.js

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ export default (
33
existingTestPlanReportsQuery,
44
getUpdateEventsQuery,
55
addTestPlansQuery,
6-
getAutomationSupportedAtVersionsQuery
6+
getAutomationSupportedAtVersionsQuery,
7+
getRerunnableReportsCountQuery
78
) => [
89
{
910
request: {
@@ -137,6 +138,19 @@ export default (
137138
}
138139
}
139140
},
141+
{
142+
request: {
143+
query: addTestPlansQuery,
144+
variables: {
145+
testPlanVersionPhases: ['DRAFT', 'CANDIDATE', 'RECOMMENDED']
146+
}
147+
},
148+
result: {
149+
data: {
150+
testPlans: []
151+
}
152+
}
153+
},
140154
{
141155
request: {
142156
query: getAutomationSupportedAtVersionsQuery,
@@ -172,5 +186,35 @@ export default (
172186
]
173187
}
174188
}
189+
},
190+
{
191+
request: {
192+
query: getRerunnableReportsCountQuery,
193+
variables: {
194+
atVersionId: '1'
195+
}
196+
},
197+
result: {
198+
data: {
199+
rerunnableReports: {
200+
previousVersionGroups: []
201+
}
202+
}
203+
}
204+
},
205+
{
206+
request: {
207+
query: getRerunnableReportsCountQuery,
208+
variables: {
209+
atVersionId: '2'
210+
}
211+
},
212+
result: {
213+
data: {
214+
rerunnableReports: {
215+
previousVersionGroups: []
216+
}
217+
}
218+
}
175219
}
176220
];

client/tests/__mocks__/GraphQLMocks/index.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ import { TEST_PLAN_REPORT_AT_BROWSER_QUERY } from '@components/common/AssignTest
66
import { TEST_PLAN_REPORT_STATUS_DIALOG_QUERY } from '@components/TestPlanReportStatusDialog/queries';
77
import { EXISTING_TEST_PLAN_REPORTS } from '@components/AddTestToQueueWithConfirmation/queries';
88
import { ME_QUERY } from '@components/App/queries';
9-
import { GET_UPDATE_EVENTS } from '@components/ReportRerun/queries';
10-
import { GET_AUTOMATION_SUPPORTED_AT_VERSIONS } from '@components/ReportRerun/queries';
9+
import {
10+
GET_UPDATE_EVENTS,
11+
GET_AUTOMATION_SUPPORTED_AT_VERSIONS,
12+
GET_RERUNNABLE_REPORTS_COUNT_QUERY
13+
} from '@components/ReportRerun/queries';
1114

1215
import TestQueuePageAdminNotPopulatedMock from './TestQueuePageAdminNotPopulatedMock';
1316
import TestQueuePageTesterNotPopulatedMock from './TestQueuePageTesterNotPopulatedMock';
@@ -26,7 +29,8 @@ export const TEST_QUEUE_PAGE_BASE_MOCK_DATA = TestQueuePageBaseMock(
2629
EXISTING_TEST_PLAN_REPORTS,
2730
GET_UPDATE_EVENTS,
2831
ADD_TEST_PLANS_QUERY,
29-
GET_AUTOMATION_SUPPORTED_AT_VERSIONS
32+
GET_AUTOMATION_SUPPORTED_AT_VERSIONS,
33+
GET_RERUNNABLE_REPORTS_COUNT_QUERY
3034
);
3135

3236
export const TEST_PLAN_REPORT_STATUS_DIALOG_MOCK_DATA =

client/tests/e2e/snapshots/saved/_test-queue.html

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,6 @@ <h3 class="disclosure-heading">
12081208
>Start NVDA Bot Run</button
12091209
><button
12101210
type="button"
1211-
disabled=""
12121211
class="btn btn-secondary">
12131212
<svg
12141213
aria-hidden="true"
@@ -1452,7 +1451,6 @@ <h3 class="disclosure-heading">
14521451
>Start JAWS Bot Run</button
14531452
><button
14541453
type="button"
1455-
disabled=""
14561454
class="btn btn-secondary">
14571455
<svg
14581456
aria-hidden="true"
@@ -1607,7 +1605,6 @@ <h3 class="disclosure-heading">
16071605
>Start NVDA Bot Run</button
16081606
><button
16091607
type="button"
1610-
disabled=""
16111608
class="btn btn-secondary">
16121609
<svg
16131610
aria-hidden="true"
@@ -1764,7 +1761,6 @@ <h3 class="disclosure-heading">
17641761
>Start VoiceOver Bot Run</button
17651762
><button
17661763
type="button"
1767-
disabled=""
17681764
class="btn btn-secondary">
17691765
<svg
17701766
aria-hidden="true"
@@ -1921,7 +1917,6 @@ <h3 class="disclosure-heading">
19211917
>Start VoiceOver Bot Run</button
19221918
><button
19231919
type="button"
1924-
disabled=""
19251920
class="btn btn-secondary">
19261921
<svg
19271922
aria-hidden="true"

server/models/services/ReviewerStatusService.js

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -332,16 +332,31 @@ const createOrUpdateReviewerStatus = async ({
332332

333333
await ReviewerStatus.upsert(upsertValues, { transaction });
334334

335-
return getReviewerStatusById({
336-
testPlanReportId,
337-
userId,
338-
reviewerStatusAttributes,
339-
testPlanReportAttributes,
340-
testPlanVersionAttributes,
341-
userAttributes,
342-
vendorAttributes,
343-
transaction
344-
});
335+
// Try to get the updated reviewer status
336+
// If the transaction was rolled back (e.g., due to an error in a nested operation),
337+
// catch the error and return the existing status or null
338+
try {
339+
return await getReviewerStatusById({
340+
testPlanReportId,
341+
userId,
342+
reviewerStatusAttributes,
343+
testPlanReportAttributes,
344+
testPlanVersionAttributes,
345+
userAttributes,
346+
vendorAttributes,
347+
transaction
348+
});
349+
} catch (error) {
350+
// If the transaction was rolled back, we can't query with it
351+
// Return the existing status if we have it, otherwise null
352+
if (
353+
error.message &&
354+
error.message.includes('rollback has been called on this transaction')
355+
) {
356+
return existing || null;
357+
}
358+
throw error;
359+
}
345360
};
346361

347362
module.exports = {

server/resolvers/addViewerResolver.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ const addViewerResolver = async (_, { testId, testPlanReportId }, context) => {
2424
transaction
2525
});
2626
} catch (error) {
27+
// Log the error but don't fail the mutation - this is a non-critical operation
28+
// that tracks which tests a reviewer has viewed. If it fails, we still want to
29+
// return the user successfully. This is intentionally a best-effort operation
2730
console.error('addViewerResolver.error', error);
2831
}
2932

0 commit comments

Comments
 (0)