segment datasets into mandated and ODP on org overview page#645
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
rosado
force-pushed
the
rosado/610-segment-datasets
branch
from
b727a61 to
f6c6099
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (9)
src/views/organisations/overview.html (1)
9-46: Add ARIA attributes for improved accessibilityThe list items could benefit from additional accessibility attributes to enhance screen reader support.
Apply these changes to improve accessibility:
-<li class="govuk-task-list__item govuk-task-list__item--with-link" data-dataset="{{ dataset.slug }}"> +<li class="govuk-task-list__item govuk-task-list__item--with-link" + data-dataset="{{ dataset.slug }}" + aria-labelledby="dataset-{{ dataset.slug }}-title" + role="listitem"> <div class="govuk-task-list__name-and-hint"> - <h2 class="govuk-heading-m"> + <h2 class="govuk-heading-m" id="dataset-{{ dataset.slug }}-title">src/middleware/overview.middleware.js (1)
119-124: Enhance function documentationThe JSDoc documentation could be more comprehensive:
- Missing function description
- Incomplete parameter documentation
- Missing @returns annotation
Consider this improved documentation:
/** + * Prepares template parameters for the organisation overview page by processing + * the LPA overview data and provisions data. * - * @param {{ provisions: { dataset: string, provision_reason: string}[], lpaOverview: Object, orgInfo: Object }} req + * @param {Object} req - Express request object + * @param {Array<{dataset: string, provision_reason: string}>} req.provisions - Dataset provisions + * @param {Object} req.lpaOverview - LPA overview data + * @param {Object} req.orgInfo - Organisation information + * @param {Object} res - Express response object + * @param {Function} next - Express next middleware function + * @returns {void} */test/unit/lpaOverviewPage.test.js (5)
14-31: Consider adding error handling to the helper function.The
datasetGrouphelper function is well-documented and follows good practices with data attributes. However, it could benefit from additional error handling.Consider adding these checks:
const datasetGroup = ({ expect }, key, datasets, document) => { + if (!document) { + throw new Error('Document is required for dataset group verification') + } + if (!datasets) { + throw new Error('Datasets array is required for verification') + } const datasetCards = document.querySelectorAll(`ul[data-reason="${key}"] li`) expect(datasetCards.length).toEqual(datasets.length)
35-35: Consider using a proper test logger.The debug statement is helpful for development but could be improved by using a proper test logger that can be controlled via environment variables.
Consider this approach:
- console.debug(`mocked datasets: statutory = ${params.datasets.statutory?.length ?? 'none'}, other = ${params.datasets.other?.length ?? 'none'}`) + const testLogger = process.env.TEST_DEBUG === 'true' ? console.debug : () => {} + testLogger(JSON.stringify({ + type: 'dataset_count', + statutory: params.datasets.statutory?.length ?? 0, + other: params.datasets.other?.length ?? 0 + }))
63-72: Add edge cases to dataset group tests.The tests handle the basic cases well, but consider adding edge cases to improve coverage.
Consider adding these test cases:
it('handles empty statutory datasets gracefully', () => { const emptyParams = { ...params, datasets: { statutory: [], other: params.datasets.other } } const html = nunjucks.render('organisations/overview.html', emptyParams) const doc = new jsdom.JSDOM(html).window.document const statutorySection = doc.querySelector('ul[data-reason="statutory"]') expect(statutorySection).toBeTruthy() expect(statutorySection.children.length).toBe(0) }) it('handles missing dataset groups gracefully', () => { const incompleteParams = { ...params, datasets: { statutory: params.datasets.statutory } } const html = nunjucks.render('organisations/overview.html', incompleteParams) const doc = new jsdom.JSDOM(html).window.document const otherSection = doc.querySelector('ul[data-reason="other"]') expect(otherSection).toBeTruthy() expect(otherSection.children.length).toBe(0) })
Line range hint
75-93: Simplify hint text logic by extracting it to a helper function.The hint text determination logic is complex and could be more maintainable if extracted to a separate function.
Consider this refactor:
+const getDatasetHint = (dataset) => { + if (!dataset.status) return '' + + const hints = { + 'Not submitted': 'Data URL not submitted', + 'Needs fixing': 'in this dataset', + 'Error': dataset.error || '', + } + + if (dataset.status === 'Error' && dataset.issue_count !== undefined) { + const count = dataset.issue_count + return count === 0 ? '' : + count === 1 ? `There is 1 issue in this dataset` : + `There are ${count} issues in this dataset` + } + + return hints[dataset.status] || 'Data URL submitted' +} allDatasets.forEach((dataset, i) => { it(`dataset cards are rendered with correct hints for dataset='${dataset.slug}'`, () => { - let expectedHint = 'Data URL submitted' - if (dataset.status === 'Not submitted') { - expectedHint = 'Data URL not submitted' - } else if (dataset.status === 'Needs fixing') { - expectedHint = 'in this dataset' - } else if (dataset.status === 'Error') { - expectedHint = dataset.error || '' - } else if (dataset.status === 'Error' && dataset.issue_count === 0) { - expectedHint = `There are ${dataset.issue_count} issues in this dataset` - } else if (dataset.status === 'Error' && dataset.issue_count === 1) { - expectedHint = `There is ${dataset.issue_count} issue in this dataset` - } else if (dataset.status === 'Error' && dataset.issue_count > 1) { - expectedHint = `There are ${dataset.issue_count} issues in this dataset` - } + const expectedHint = getDatasetHint(dataset) const datasetCard = document.querySelector(`[data-dataset="${dataset.slug}"]`) expect(datasetCard.querySelector('.govuk-task-list__hint').textContent.trim()).toContain(expectedHint) })
Line range hint
114-139: Extract repeated values into constants.The code uses string literals repeatedly for URLs and selectors. Consider extracting these into constants for better maintainability.
Consider this improvement:
+const SELECTORS = { + DATASET_CARD: 'li[data-dataset]:not([data-dataset=""])', + STATUS: '.govuk-task-list__status', + LINK: '.govuk-task-list__link', + GENERIC_LINK: '.govuk-link' +} + +const URL_TEMPLATES = { + GET_STARTED: (org, slug) => `/organisations/${org}/${slug}/get-started`, + OVERVIEW: (org, slug) => `/organisations/${org}/${slug}/overview` +} -const datasetCards = document.querySelectorAll('li[data-dataset]:not([data-dataset=""])') +const datasetCards = document.querySelectorAll(SELECTORS.DATASET_CARD) allDatasets.forEach((dataset, i) => { it(`Renders the correct status on each dataset card for dataset='${dataset.slug}'`, () => { // ... existing code ... - const statusIndicator = datasetCards[i].querySelector('.govuk-task-list__status') + const statusIndicator = datasetCards[i].querySelector(SELECTORS.STATUS) // ... rest of the code ... })src/routes/schemas.js (2)
43-54: Add JSDoc documentation for the DatasetItem schemaWhilst the schema structure is well-defined, it would benefit from comprehensive documentation. Consider adding JSDoc comments to explain:
- The purpose of each field
- The relationship between
issue_countandnumIssues- When optional fields are expected to be present
Example improvement:
+/** + * Represents a dataset item with its status and associated metrics + * @typedef {Object} DatasetItem + * @property {URL} [endpoint] - The URL endpoint where the dataset can be accessed + * @property {string} status - Current status of the dataset (from datasetStatusEnum) + * @property {string} slug - Unique identifier for the dataset + * @property {number} [issue_count] - Raw count of issues detected + * @property {string} [error] - Primary error message if dataset processing failed + * @property {string} [http_error] - HTTP-specific error message + * @property {string} [issue] - Description of the current issue + * @property {number} [entity_count] - Number of entities in the dataset + * @property {number} [numIssues] - User-friendly aggregated issue count + */ const DatasetItem = v.strictObject({
58-61: Add validation for minimum dataset requirementsBased on the PR objectives, LPAs are legally required to provide certain datasets. Consider adding validation to ensure the
statutoryarray contains the mandatory datasets:datasets: v.object({ - statutory: v.optional(v.array(DatasetItem)), + statutory: v.pipe( + v.array(DatasetItem), + v.custom((items) => { + const requiredDatasets = ['brownfield-land', 'developer-contributions']; + return requiredDatasets.every(dataset => + items.some(item => item.slug === dataset) + ); + }, 'Missing required statutory datasets') + ), other: v.optional(v.array(DatasetItem)) }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/middleware/overview.middleware.js(5 hunks)src/routes/schemas.js(1 hunks)src/views/organisations/overview.html(2 hunks)test/unit/lpaOverviewPage.test.js(4 hunks)test/unit/middleware/overview.middleware.test.js(2 hunks)
🔇 Additional comments (5)
src/views/organisations/overview.html (1)
Line range hint 1-4: Verify all required GOV.UK components are imported
Let's ensure all used GOV.UK components are properly imported.
✅ Verification successful
All required GOV.UK components are properly imported
The verification shows perfect alignment between component usage and imports:
govukBreadcrumbsis used and imported from "govuk/components/breadcrumbs/macro"govukTagis used and imported from "govuk/components/tag/macro"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for missing GOV.UK component imports
# Find all GOV.UK component usage in the template
echo "Checking component usage..."
rg -o 'govuk\w+' src/views/organisations/overview.html
# Find all component imports
echo "Checking component imports..."
rg -o 'from "govuk/components/.*?/macro' src/views/organisations/overview.html
Length of output: 397
test/unit/middleware/overview.middleware.test.js (1)
Line range hint 1-133: Verify test coverage for dataset categorisation
Let's ensure we have adequate test coverage for the new dataset categorisation functionality.
✅ Verification successful
Let me try a different approach to verify the test coverage.
Test coverage appears adequate for dataset categorisation
The implementation shows that dataset categorisation is properly tested in the provided test file:
- The
prepareOverviewTemplateParamstest verifies the basic template rendering - The
aggregateOverviewDatatests cover various scenarios including empty input, issue counting, and status handling - The code categorises datasets into 'statutory' and 'other' groups, which is reflected in the template rendering test
The test coverage aligns well with the implementation in overview.middleware.js, particularly:
- Dataset aggregation and grouping
- Status ordering and issue counting
- Template parameter preparation with proper categorisation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check test coverage and identify missing test cases
# Look for test cases related to dataset categorisation
echo "Checking for dataset categorisation test cases..."
rg -l "statutory|mandated|ODP" "test/**/*.test.js"
# Look for provision-related test cases
echo "Checking for provision-related test cases..."
rg -l "provision" "test/**/*.test.js"
# Check for dataset-specific test cases
echo "Checking for specific dataset test cases..."
rg -l "brownfield-land|developer-contributions|conservation-area|article-4|tree-preservation" "test/**/*.test.js"
# Look for potential missing test files
echo "Checking for potential missing test files..."
fd -e js -e ts src/middleware/provision | sed 's/src/test\/unit/g' | sed 's/\.js/\.test.js/g' | sed 's/\.ts/\.test.ts/g'
Length of output: 907
Script:
#!/bin/bash
# Check test files in the test directory
echo "Listing test files..."
fd -e test.js test/
# Check overview middleware implementation
echo "\nChecking overview middleware implementation..."
cat src/middleware/overview.middleware.js
# Look for dataset categorisation in source
echo "\nLooking for dataset categorisation in source..."
rg -A 5 "dataSubjects|statutory|mandated|ODP" src/
Length of output: 11929
src/middleware/overview.middleware.js (2)
195-195: LGTM: Correct placement in middleware pipeline
The fetchProvisions middleware is correctly placed before prepareOverviewTemplateParams where the provisions data is needed.
150-167: 🛠️ Refactor suggestion
Improve provisions data processing logic
The current implementation could be enhanced in several ways:
- The switch statement is overly complex for a binary choice
- Missing explicit handling of all possible provision reasons
- Lack of validation for provision_reason values
- Multiple iterations over the datasets collection
Consider this more efficient implementation:
- const provisionData = new Map()
- for (const provision of provisions ?? []) {
- provisionData.set(provision.dataset, provision.provision_reason)
- }
-
- const datasetsByReason = _.groupBy(datasets, (ds) => {
- const reason = provisionData.get(ds.slug)
- switch (reason) {
- case 'statutory':
- return 'statutory'
- default:
- return 'other'
- }
- })
-
- for (const coll of Object.values(datasetsByReason)) {
- coll.sort((a, b) => a.slug.localeCompare(b.slug))
- }
+ const provisionData = new Map(
+ (provisions ?? []).map(p => [p.dataset, p.provision_reason])
+ );
+
+ const datasetsByReason = {
+ statutory: [],
+ other: []
+ };
+
+ // Single pass through datasets
+ datasets.forEach(ds => {
+ const reason = provisionData.get(ds.slug);
+ const category = reason === 'statutory' ? 'statutory' : 'other';
+ datasetsByReason[category].push(ds);
+ });
+
+ // Sort both arrays
+ Object.values(datasetsByReason).forEach(collection =>
+ collection.sort((a, b) => a.slug.localeCompare(b.slug))
+ );Let's verify all possible provision reason values:
src/routes/schemas.js (1)
58-61: 🛠️ Refactor suggestion
Verify template handling of optional arrays
The statutory and other arrays are marked as optional, which could lead to undefined checks in templates. Ensure that the template code in organisations/overview.html properly handles cases where either array is undefined.
Additionally, consider adding default empty arrays to ensure consistent template rendering:
datasets: v.object({
- statutory: v.optional(v.array(DatasetItem)),
- other: v.optional(v.array(DatasetItem))
+ statutory: v.withDefault(v.array(DatasetItem), []),
+ other: v.withDefault(v.array(DatasetItem), [])
}),✅ Verification successful
Let me verify the template's handling of undefined arrays by checking the schema implementation.
Template handling of optional arrays is safe, but default values would improve code clarity
The template safely handles undefined arrays through:
- Length checks before rendering:
{% if datasets.statutory.length > 0 %} - Tests show null-safe access with optional chaining:
datasets.statutory?.length - Test suite validates both undefined and populated scenarios
However, the suggestion to use withDefault would indeed improve code clarity and reduce the need for defensive programming. The current implementation is functionally correct but could be more explicit.
Recommended change remains valid:
datasets: v.object({
- statutory: v.optional(v.array(DatasetItem)),
- other: v.optional(v.array(DatasetItem))
+ statutory: v.withDefault(v.array(DatasetItem), []),
+ other: v.withDefault(v.array(DatasetItem), [])
}),🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check template handling of undefined arrays
# Look for null checks or default values in the template
rg -A 2 'statutory|other' src/views/organisations/overview.html
Length of output: 743
Script:
#!/bin/bash
# Check the schema implementation for DatasetItem and any default values
ast-grep --pattern 'const DatasetItem = $_'
Length of output: 827
Script:
#!/bin/bash
# Look for any validation or transformation of datasets before template rendering
rg -A 5 'datasets.*statutory|datasets.*other' --type js
Length of output: 1896
|
I made a mistake on one of the requirements: If the LPA is part of the ODP programme (signified by the Open Digital Planning in the project column), then we should display the additional message:
As this requirement states, it should check whether or not the LPA is part of the ODP, but what I missed out is that it should then change the content based on that - i.e. If the LPA is part of the programme:
.. and if they are not part of the programme:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
src/middleware/overview.middleware.js (3)
119-124: Enhance JSDoc documentationThe JSDoc could be more comprehensive:
- Add a description of the function's purpose
- Specify the return type
- Use more specific types for the provisions array elements
/** + * Prepares template parameters for the organisation overview page by processing + * the LPA overview data and provisions data. * * @param {{ provisions: { dataset: string, provision_reason: 'statutory' | 'other', project: string }[], lpaOverview: Object, orgInfo: Object }} req * @param {Response} res * @param {NextFunction} next + * @returns {void} */
156-164: Simplify dataset grouping logicThe switch statement with a single case can be simplified, and error handling should be added for invalid provision reasons.
- const datasetsByReason = _.groupBy(datasets, (ds) => { - const reason = provisionData.get(ds.slug)?.provision_reason - switch (reason) { - case 'statutory': - return 'statutory' - default: - return 'other' - } - }) + const datasetsByReason = _.groupBy(datasets, (ds) => { + const reason = provisionData.get(ds.slug)?.provision_reason + if (!reason || !['statutory', 'other'].includes(reason)) { + console.warn(`Invalid provision reason "${reason}" for dataset "${ds.slug}"`) + } + return reason === 'statutory' ? 'statutory' : 'other' + })
166-168: Avoid in-place sorting of collectionsCreate a new sorted array instead of modifying the original collection to prevent potential side effects if the data is reused elsewhere.
- for (const coll of Object.values(datasetsByReason)) { - coll.sort((a, b) => a.slug.localeCompare(b.slug)) - } + Object.keys(datasetsByReason).forEach(key => { + datasetsByReason[key] = [...datasetsByReason[key]].sort((a, b) => + a.slug.localeCompare(b.slug) + ) + })test/unit/lpaOverviewPage.test.js (3)
14-31: Consider adding accessibility-related assertionsThe helper function effectively verifies dataset cards and titles. Consider enhancing it to verify ARIA attributes and other accessibility features, ensuring the grouped datasets are properly announced to screen readers.
Add these assertions to the helper function:
const datasetGroup = ({ expect }, key, datasets, document) => { const datasetCards = document.querySelectorAll(`ul[data-reason="${key}"] li`) expect(datasetCards.length).toEqual(datasets.length) const datasetSlugToReadableName = makeDatasetSlugToReadableNameFilter(datasetNameMapping) datasets.forEach((dataset, i) => { expect(datasetCards[i].querySelector('.govuk-heading-m').textContent).toContain(datasetSlugToReadableName(dataset.slug)) + // Verify accessibility attributes + const list = document.querySelector(`ul[data-reason="${key}"]`) + expect(list.getAttribute('role')).toBe('list') + expect(list.getAttribute('aria-label')).toContain(key === 'statutory' ? 'Mandatory' : 'ODP') }) }
Line range hint
78-96: Refactor hint text determination logicThe current implementation has duplicated conditions and complex nesting. Consider extracting the hint text logic into a separate function for better maintainability.
Extract the hint text logic:
+const getExpectedHint = (dataset) => { + if (!dataset.status) return '' + + const hints = { + 'Not submitted': 'Data URL not submitted', + 'Needs fixing': 'in this dataset', + 'Submitted': 'Data URL submitted' + } + + if (dataset.status === 'Error') { + if (dataset.error) return dataset.error + const count = dataset.issue_count || 0 + if (count === 0) return '' + return `There ${count === 1 ? 'is' : 'are'} ${count} issue${count === 1 ? '' : 's'} in this dataset` + } + + return hints[dataset.status] || '' +} allDatasets.forEach((dataset, i) => { it(`dataset cards are rendered with correct hints for dataset='${dataset.slug}'`, () => { - let expectedHint = 'Data URL submitted' - if (dataset.status === 'Not submitted') { - expectedHint = 'Data URL not submitted' - } else if (dataset.status === 'Needs fixing') { - expectedHint = 'in this dataset' - } else if (dataset.status === 'Error') { - expectedHint = dataset.error || '' - } else if (dataset.status === 'Error' && dataset.issue_count === 0) { - expectedHint = `There are ${dataset.issue_count} issues in this dataset` - } else if (dataset.status === 'Error' && dataset.issue_count === 1) { - expectedHint = `There is ${dataset.issue_count} issue in this dataset` - } else if (dataset.status === 'Error' && dataset.issue_count > 1) { - expectedHint = `There are ${dataset.issue_count} issues in this dataset` - } + const expectedHint = getExpectedHint(dataset) const datasetCard = document.querySelector(`[data-dataset="${dataset.slug}"]`) expect(datasetCard.querySelector('.govuk-task-list__hint').textContent.trim()).toContain(expectedHint) }) })
117-118: Extract selector as a constantThe dataset card selector is used multiple times. Consider extracting it as a constant for better maintainability and consistency.
Extract the selector:
+const DATASET_CARD_SELECTOR = 'li[data-dataset]:not([data-dataset=""])' -const datasetCards = document.querySelectorAll('li[data-dataset]:not([data-dataset=""])') +const datasetCards = document.querySelectorAll(DATASET_CARD_SELECTOR)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/middleware/overview.middleware.js(5 hunks)src/routes/schemas.js(1 hunks)src/views/organisations/overview.html(2 hunks)test/unit/lpaOverviewPage.test.js(4 hunks)test/unit/middleware/overview.middleware.test.js(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/routes/schemas.js
- src/views/organisations/overview.html
- test/unit/middleware/overview.middleware.test.js
🔇 Additional comments (2)
src/middleware/overview.middleware.js (2)
38-45: SQL injection vulnerability still present
The current implementation remains vulnerable to SQL injection attacks through string interpolation in the SQL query.
196-196: LGTM: Correct positioning in middleware pipeline
The fetchProvisions middleware is correctly positioned before prepareOverviewTemplateParams which depends on its data.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/middleware/overview.middleware.js (2)
119-124: Enhance function documentationThe JSDoc could be more comprehensive. Consider:
- Adding
@returnsto document the shape ofreq.templateParams- Adding
@throwsto document potential errors- Using TypeScript-style types for better clarity of the data structure
/** + * Prepares template parameters for the organisation overview page * - * @param {{ provisions: { dataset: string, provision_reason: string, project: string }[], lpaOverview: Object, orgInfo: Object }} req + * @param {{ + * provisions: Array<{ + * dataset: string, + * provision_reason: 'statutory' | 'other', + * project: string + * }>, + * lpaOverview: Object, + * orgInfo: Object + * }} req * @param {Response} res * @param {NextFunction} next + * @returns {void} + * @throws {Error} When required data is missing */
166-168: Consider case-insensitive sorting for dataset slugsThe current sorting implementation is case-sensitive. Consider using a case-insensitive comparison for more natural ordering of dataset slugs.
- coll.sort((a, b) => a.slug.localeCompare(b.slug)) + coll.sort((a, b) => a.slug.localeCompare(b.slug, undefined, { sensitivity: 'base' }))src/routes/schemas.js (2)
83-95: Enhance documentation for the DatasetItem schemaThe schema definition is well-structured, but the documentation for the
numIssuesfield could be clearer.Consider updating the comment to:
- // synthetic entry, represents a user friendly count (e.g. count missing value in a column as 1 issue) + // Represents a user-friendly issue count where each type of issue (e.g., missing values in a column) + // is counted as a single issue, regardless of how many records are affected
132-132: Document conditions for null lastUpdated valuesThe
lastUpdatedfield now accepts null values, but there's no documentation explaining when this occurs.Consider adding a comment above the field to explain the scenarios where
lastUpdatedmight be null, such as:// lastUpdated can be null for new endpoints or when update timestamp couldn't be retrieved lastUpdated: v.nullable(v.string()),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/middleware/overview.middleware.js(5 hunks)src/routes/schemas.js(2 hunks)
🔇 Additional comments (1)
src/middleware/overview.middleware.js (1)
38-45: SQL injection vulnerability still present
The previously identified SQL injection vulnerability remains unaddressed. The implementation continues to use direct string interpolation for constructing the SQL query.
5 participants
What type of PR is this? (check all applicable)
Description
The datasets on organisations' overview pages are now grouped into two groups:
Where a particular dataset falls is done by looking up values in the
provisiontable.Summary of changes
OrgOverviewPageschemadatasetsproperty to be a an object with (potentially) two fields, one for each group we currently support:statutoryandother.provisiontable to group datasetsorganisations/overviewtemplate, datasets are now grouped under two different headings (as in design)data-reasonanddata-datasetattributes to make testing easier.Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Go to any LPA's page and compare with design in #618, e.g. here
Added/updated tests?
Summary by CodeRabbit
New Features
DatasetItem, enhancing dataset representation.Bug Fixes
Tests