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.

Sign up for GitHub

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

Jump to bottom

Fix validation message #821

Closed
donyunardi opened this issue Apr 4, 2023 · 2 comments · Fixed by #877
Closed

Fix validation message #821

donyunardi opened this issue Apr 4, 2023 · 2 comments · Fixed by #877
Assignees
@chlebowa
Labels
core good first issue Good for newcomers

Comments

@donyunardi
Copy link
Contributor

donyunardi commented Apr 4, 2023 •
edited
Loading

Good findings.

Also, we should improve the validation message given out here:

image

The current message implies that all columns do not exist where it supposed to be just for TRT column.

Originally posted by @donyunardi in #819 (comment)

Acceptance Criteria

  • Validation message should only show the missing columns, not all columns selected in the shiny input.
  • Using the example above, the validation should say ADSL does not have the required variables: TRT
@donyunardi donyunardi added core good first issue Good for newcomers labels Apr 4, 2023
@asbates
Copy link
Contributor

asbates commented Apr 5, 2023

FYI this is for validate_has_variable.

@asbates asbates mentioned this issue Apr 5, 2023
Merged
@chlebowa chlebowa mentioned this issue Jul 27, 2023
Merged
@chlebowa chlebowa self-assigned this Jul 30, 2023
@chlebowa
Copy link
Contributor

Fixed here

chlebowa added a commit that referenced this issue Aug 1, 2023
@chlebowa @github-actions @gogonzo
769 replace filters argument (#877)
25fa0b2 
Fixes #769 

Argument is renamed as per request.
Downstream effects are resolved.

I also identified instances of partial name matching when indexing lists
and removed them. There might be more. This is an argument against using
`list[["name"]]` over `list$name`. The occurrence of `modules$filter`
was an unwelcome surprise.

Also fixes #821

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: go_gonzo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chlebowa chlebowa

Labels
core good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

769 replace filters argument insightsengineering/teal
3 participants
@donyunardi @asbates @chlebowa