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

Ensure column order in table view #769

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

rosado
Copy link
Collaborator

@rosado rosado commented Jan 9, 2025

What type of PR is this? (check all applicable)

  • Feature

Description

This PR ensures the "name" and "reference" columns come first in the table view.

Related Tickets & Documents

Closes #755

QA Instructions, Screenshots, Recordings

From what I can see the column order matches the desired one, but mostly by happy accident than anything else.

Added/updated tests?

  • Yes

QA sign off

  • Code has been checked and approved
  • Design has been checked and approved
  • Product and business logic has been checked and proved

Summary by CodeRabbit

  • New Features

    • Enhanced table parameter construction to prioritise specific fields ('name' and 'reference') in dataset rendering.
  • Tests

    • Added test cases to verify correct field prioritisation in table parameter generation.

These changes improve the organisation of dataset fields during table rendering, ensuring consistent and predictable column ordering.

@rosado rosado changed the title Rosado/755 column order Ensure column order in table view Jan 9, 2025
coderabbitai[bot]

This comment was marked as outdated.

@rosado rosado marked this pull request as draft January 9, 2025 13:37
@rosado rosado force-pushed the rosado/755-column-order branch from d359850 to a814866 Compare January 9, 2025 13:42
@digital-land digital-land deleted a comment from coderabbitai bot Jan 9, 2025
@digital-land digital-land deleted a comment from github-actions bot Jan 9, 2025
@rosado rosado marked this pull request as ready for review January 9, 2025 13:46
Copy link
Contributor

coderabbitai bot commented Jan 9, 2025

Walkthrough

The pull request modifies the constructTableParams middleware function to reorder dataset fields, specifically prioritising 'name' and 'reference' fields at the beginning of the columns and fields arrays. This change ensures that when rendering tables, these specified fields appear first. A corresponding unit test has been added to verify this new behaviour, checking that the 'name' field is correctly positioned as the first column.

Changes

File Change Summary
src/middleware/dataview.middleware.js Modified constructTableParams to create leadingFields with 'name' and 'reference', then construct orderedDatasetFields to prioritise these fields.
test/unit/middleware/dataview.middleware.test.js Added new test cases to validate the new field ordering in constructTableParams.

Assessment against linked issues

Objective Addressed Explanation
Prioritise ref + name first [#755]

Possibly related PRs

  • segment datasets into mandated and ODP on org overview page #645: The changes in the src/middleware/dataview.middleware.js file regarding the organization of dataset fields may relate to the modifications in the prepareOverviewTemplateParams function in the overview.middleware.js, which also involves structuring datasets into distinct categories. Both PRs focus on how datasets are organized and prioritized, although they address different aspects of dataset handling.

Suggested reviewers

  • alextea
  • DilwoarH
  • GeorgeGoodall-GovUk

Poem

🐰 Columns dance in new array,
Name and reference lead the way!
Middleware magic, fields align,
With rabbit's touch, they now refine.
Table's order, now precise and neat! 📊


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24d6a14 and e542838.

📒 Files selected for processing (2)
  • src/middleware/dataview.middleware.js (1 hunks)
  • test/unit/middleware/dataview.middleware.test.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/unit/middleware/dataview.middleware.test.js
  • src/middleware/dataview.middleware.js
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: run-tests / test

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@rosado rosado self-assigned this Jan 9, 2025
@rosado rosado added the enhancement New feature or request label Jan 9, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/unit/middleware/dataview.middleware.test.js (1)

122-122: Add additional test cases for edge scenarios

The current test suite would benefit from additional test cases to verify:

  1. Behaviour when neither 'name' nor 'reference' fields exist
  2. Behaviour when only one of the fields exists
  3. Order preservation of remaining fields

Here's an example of additional test cases:

it('preserves existing order when name and reference fields are absent', () => {
  const req = {
    entities: [{ id: 1, foo: 'bar', baz: 'qux' }],
    uniqueDatasetFields: ['foo', 'baz']
  }
  const res = {}
  const next = vi.fn()

  constructTableParams(req, res, next)

  expect(req.tableParams.columns).toEqual(['foo', 'baz'])
  expect(req.tableParams.fields).toEqual(['foo', 'baz'])
})

it('handles single leading field correctly', () => {
  const req = {
    entities: [{ id: 1, name: 'First', foo: 'bar' }],
    uniqueDatasetFields: ['foo', 'name']
  }
  const res = {}
  const next = vi.fn()

  constructTableParams(req, res, next)

  expect(req.tableParams.columns).toEqual(['name', 'foo'])
  expect(req.tableParams.fields).toEqual(['name', 'foo'])
})
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ac897e and a814866.

📒 Files selected for processing (2)
  • src/middleware/dataview.middleware.js (1 hunks)
  • test/unit/middleware/dataview.middleware.test.js (1 hunks)
🔇 Additional comments (1)
test/unit/middleware/dataview.middleware.test.js (1)

110-122: Fix typo and enhance test coverage

The test case has the same issues as previously identified:

  1. Typo in the description: 'refernce' should be 'reference'
  2. Test should verify both 'name' and 'reference' columns
  3. Test data could be more comprehensive

Apply the suggested fix from the previous review to address these issues.

src/middleware/dataview.middleware.js Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Jan 9, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 69.09% 5432 / 7862
🔵 Statements 69.09% 5432 / 7862
🔵 Functions 67.18% 215 / 320
🔵 Branches 81.79% 692 / 846
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/middleware/dataview.middleware.js 100% 100% 50% 100%
Generated in workflow #618 for commit e542838 by the Vitest Coverage Report Action

@rosado rosado requested a review from DilwoarH January 9, 2025 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tables: Order columns based on specification
2 participants