Skip to content

Ensures missing fields validation shows row ref - #178

Open
jwilson232 wants to merge 1 commit into
mainfrom
jw/missing-fields-feedback
Open

Ensures missing fields validation shows row ref#178
jwilson232 wants to merge 1 commit into
mainfrom
jw/missing-fields-feedback

Conversation

@jwilson232

Copy link
Copy Markdown
Contributor

A bug in the way that validation errors where
being grouped meant that the 'First 3 examples'
did not render properly. The logic to render the
row ref has been updated to show for error counts
greater than or equal to one.

@coveralls

coveralls commented Aug 11, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 32263658452

Coverage remained the same at 81.013%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 79
Covered Lines: 64
Line Coverage: 81.01%
Coverage Strength: 0.81 hits per line

💛 - Coveralls

@jwilson232

Copy link
Copy Markdown
Contributor Author

resolves #79

values, openpyxl_workbook
)
if (file_type in ["xlsx", "csv"] and "grants/" in error["path_no_number"])
if (file_type in ["xlsx", "csv"] and error["path_no_number"].split("/")[0] == "grants")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we sure that all instances of error["path_no_number"] start with grants/? This change will mean that anything with, for example, hello/grants will not satisfy this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@chrisarridge Will the error["path_no_number"] not always be something like grants/fundingOrganization or grants as per https://github.com/ThreeSixtyGiving/standard/blob/main/schema/360-giving-package-schema.json? I think the logic was failing previously as "grants/" in error["path_no_number"]) was failing to match against just grants.

@jwilson232 jwilson232 Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could switch the logic to .contains("grants") or .startswith("grants) but not sure if that is correct?

@chrisarridge chrisarridge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One comment to check a change in logic, and two additional quick things:

  1. Please could you add a before/after screenshot to the PR description so we can see the visual change.
  2. Could you add a prefix to the commit message, e.g., "cove: ensure missing fields validation shows row ref" (or similar)

A bug in the way that validation errors where
being grouped meant that the 'First 3 examples'
did not render properly. The logic to render the
row ref has been updated to show for error counts
greater than or equal to one.
@jwilson232
jwilson232 force-pushed the jw/missing-fields-feedback branch from e279180 to 9a6f8c0 Compare August 19, 2026 14:22
@jwilson232

Copy link
Copy Markdown
Contributor Author

3 or less errors:

Before:
before-3-or-less
After:
after-3-or-less

4 or more errors:

Before:
before-4-or-more

After
Screenshot 2026-08-19 at 15 26 25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants