-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Reporting] Fix and test for Listing of Reports #74453
Changes from all commits
c2415a1
cb6dd51
e60f215
a39f8af
44df9ce
2609d03
d0215ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,13 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { | |
router.get( | ||
{ | ||
path: `${MAIN_ENTRY}/list`, | ||
validate: false, | ||
validate: { | ||
query: schema.object({ | ||
page: schema.string({ defaultValue: '0' }), | ||
size: schema.string({ defaultValue: '10' }), | ||
ids: schema.maybe(schema.string()), | ||
}), | ||
}, | ||
}, | ||
userHandler(async (user, context, req, res) => { | ||
// ensure the async dependencies are loaded | ||
|
@@ -50,7 +56,7 @@ export function registerJobInfoRoutes(reporting: ReportingCore) { | |
page: queryPage = '0', | ||
size: querySize = '10', | ||
ids: queryIds = null, | ||
} = req.query as ListQuery; | ||
} = req.query as ListQuery; // NOTE: type inference is not working here. userHandler breaks it? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to understand why the type of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to know as well. I spent a good deal of time trying to get types to pass through with the higher order handler (https://github.com/elastic/kibana/blob/master/x-pack/plugins/reporting/server/routes/lib/authorized_user_pre_routing.ts#L15), but there's some interesting effects it might have trying to get type information to pass through properly. This was a pretty big change in our codebase to get everything working properly, as a lot of our utilities relied on augmented Hapi request objects since we used the |
||
const page = parseInt(queryPage, 10) || 0; | ||
const size = Math.min(100, parseInt(querySize, 10) || 10); | ||
const jobIds = queryIds ? queryIds.split(',') : null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,6 @@ import { FtrProviderContext } from '../../ftr_provider_context'; | |
export default ({ loadTestFile }: FtrProviderContext) => { | ||
describe('reporting management app', function () { | ||
this.tags('ciGroup7'); | ||
loadTestFile(require.resolve('./report_delete_pagination')); | ||
loadTestFile(require.resolve('./report_listing')); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. renamed the file since it tests more than just deleting a report in the listing |
||
}); | ||
}; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import expect from '@kbn/expect'; | ||
import { WebElementWrapper } from 'test/functional/services/lib/web_element_wrapper'; | ||
import { FtrProviderContext } from '../../ftr_provider_context'; | ||
|
||
const getTableTextFromElement = async (tableEl: WebElementWrapper) => { | ||
const rows = await tableEl.findAllByCssSelector('tbody tr'); | ||
return ( | ||
await Promise.all( | ||
rows.map(async (row) => { | ||
return await row.getVisibleText(); | ||
}) | ||
) | ||
).join('\n'); | ||
}; | ||
|
||
export default ({ getPageObjects, getService }: FtrProviderContext) => { | ||
const pageObjects = getPageObjects(['common', 'reporting']); | ||
const log = getService('log'); | ||
const retry = getService('retry'); | ||
const security = getService('security'); | ||
|
||
const testSubjects = getService('testSubjects'); | ||
const findInstance = getService('find'); | ||
const esArchiver = getService('esArchiver'); | ||
|
||
describe('Listing of Reports', function () { | ||
before(async () => { | ||
await security.testUser.setRoles(['kibana_admin', 'reporting_user']); | ||
await esArchiver.load('empty_kibana'); | ||
}); | ||
|
||
beforeEach(async () => { | ||
// to reset the data after deletion testing | ||
await esArchiver.load('reporting/archived_reports'); | ||
await pageObjects.common.navigateToApp('reporting'); | ||
await testSubjects.existOrFail('reportJobListing', { timeout: 200000 }); | ||
}); | ||
|
||
after(async () => { | ||
await esArchiver.unload('empty_kibana'); | ||
await security.testUser.restoreDefaults(); | ||
}); | ||
|
||
afterEach(async () => { | ||
await esArchiver.unload('reporting/archived_reports'); | ||
}); | ||
|
||
it('Confirm single report deletion works', async () => { | ||
log.debug('Checking for reports.'); | ||
await retry.try(async () => { | ||
await testSubjects.click('checkboxSelectRow-k9a9xlwl0gpe1457b10rraq3'); | ||
}); | ||
const deleteButton = await testSubjects.find('deleteReportButton'); | ||
await retry.waitFor('delete button to become enabled', async () => { | ||
return await deleteButton.isEnabled(); | ||
}); | ||
await deleteButton.click(); | ||
await testSubjects.exists('confirmModalBodyText'); | ||
await testSubjects.click('confirmModalConfirmButton'); | ||
await retry.try(async () => { | ||
await testSubjects.waitForDeleted('checkboxSelectRow-k9a9xlwl0gpe1457b10rraq3'); | ||
}); | ||
}); | ||
|
||
it('Paginates content', async () => { | ||
const previousButton = await testSubjects.find('pagination-button-previous'); | ||
|
||
// previous CAN NOT be clicked | ||
expect(await previousButton.getAttribute('disabled')).to.be('true'); | ||
|
||
// scan page 1 | ||
let tableText = await getTableTextFromElement(await testSubjects.find('reportJobListing')); | ||
const PAGE_CONTENT_1 = `[Logs] File Type Scatter Plot\nvisualization\n2020-04-21 @ 07:01 PM\ntest_user\nCompleted at 2020-04-21 @ 07:02 PM | ||
[Logs] File Type Scatter Plot\nvisualization\n2020-04-21 @ 07:01 PM\ntest_user\nCompleted at 2020-04-21 @ 07:02 PM | ||
[Logs] Heatmap\nvisualization\n2020-04-21 @ 07:00 PM\ntest_user\nCompleted at 2020-04-21 @ 07:01 PM | ||
[Logs] Heatmap\nvisualization\n2020-04-21 @ 07:00 PM\ntest_user\nCompleted at 2020-04-21 @ 07:01 PM | ||
[Flights] Flight Delays\nvisualization\n2020-04-21 @ 07:00 PM\ntest_user\nCompleted at 2020-04-21 @ 07:01 PM | ||
[Flights] Flight Delays\nvisualization\n2020-04-21 @ 07:00 PM\ntest_user\nCompleted at 2020-04-21 @ 07:01 PM | ||
pdf\ndashboard\n2020-04-21 @ 07:00 PM\ntest_user\nCompleted at 2020-04-21 @ 07:00 PM | ||
pdf\ndashboard\n2020-04-21 @ 07:00 PM\ntest_user\nCompleted at 2020-04-21 @ 07:00 PM | ||
[Flights] Flight Cancellations\nvisualization\n2020-04-21 @ 06:59 PM\ntest_user\nCompleted at 2020-04-21 @ 07:00 PM | ||
[Flights] Markdown Instructions\nvisualization\n2020-04-21 @ 06:59 PM\ntest_user\nCompleted at 2020-04-21 @ 07:00 PM`; | ||
expect(tableText).to.be(PAGE_CONTENT_1); | ||
|
||
// click page 2 | ||
await testSubjects.click('pagination-button-1'); | ||
await findInstance.byCssSelector('[data-test-page="1"]'); | ||
|
||
// previous CAN be clicked | ||
expect(await previousButton.getAttribute('disabled')).to.be(null); | ||
|
||
// scan page 2 | ||
tableText = await getTableTextFromElement(await testSubjects.find('reportJobListing')); | ||
const PAGE_CONTENT_2 = `[eCommerce] Revenue Tracking\ncanvas workpad\n2020-04-21 @ 06:58 PM\ntest_user\nCompleted at 2020-04-21 @ 06:59 PM | ||
[Logs] Web Traffic\ncanvas workpad\n2020-04-21 @ 06:58 PM\ntest_user\nCompleted at 2020-04-21 @ 06:59 PM | ||
[Flights] Overview\ncanvas workpad\n2020-04-21 @ 06:58 PM\ntest_user\nCompleted at 2020-04-21 @ 06:59 PM | ||
[eCommerce] Revenue Dashboard\ndashboard\n2020-04-21 @ 06:57 PM\ntest_user\nCompleted at 2020-04-21 @ 06:58 PM | ||
[Logs] Web Traffic\ndashboard\n2020-04-21 @ 06:57 PM\ntest_user\nCompleted at 2020-04-21 @ 06:58 PM | ||
[Flights] Global Flight Dashboard\ndashboard\n2020-04-21 @ 06:56 PM\ntest_user\nCompleted at 2020-04-21 @ 06:57 PM | ||
[Flights] Global Flight Dashboard\ndashboard\n2020-04-21 @ 06:56 PM\ntest_user\nCompleted at 2020-04-21 @ 06:57 PM | ||
report4csv\n2020-04-21 @ 06:55 PM\ntest_user\nCompleted at 2020-04-21 @ 06:56 PM - Max size reached\nreport3csv\n2020-04-21 @ 06:55 PM | ||
test_user\nCompleted at 2020-04-21 @ 06:55 PM - Max size reached\nreport2csv\n2020-04-21 @ 06:54 PM\ntest_user\nCompleted at 2020-04-21 @ 06:55 PM - Max size reached`; | ||
expect(tableText).to.be(PAGE_CONTENT_2); | ||
|
||
// click page 3 | ||
await testSubjects.click('pagination-button-2'); | ||
await findInstance.byCssSelector('[data-test-page="2"]'); | ||
|
||
// scan page 3 | ||
tableText = await getTableTextFromElement(await testSubjects.find('reportJobListing')); | ||
const PAGE_CONTENT_3 = `report1csv\n2020-04-21 @ 06:54 PM\ntest_user\nCompleted at 2020-04-21 @ 06:54 PM - Max size reached`; | ||
expect(tableText).to.be(PAGE_CONTENT_3); | ||
}); | ||
}); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note from @restrry: "platform doesn’t provide request params / body / query unless validation schema is provided"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of remarks
number
, you parse them belowuserHandler
might break type inference logic, at least it doesn't implement the same interface as wrappers in docs https://github.com/elastic/kibana/blob/e57f4d2b25ac95183e876ea0c305138ab119ea26/src/core/MIGRATION_EXAMPLES.md#migrating-hapi-pre-handlers.I'd recommend either implement it as RequestHandlerWrapper or pass user via
RequestHandlerContext
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our HoH has roughly the same type passthrough interface. Is there something obvious I'm missing?