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

FI-3178: Validate Resource Without Validation Messages #595

Merged

Conversation

vanessuniq
Copy link
Contributor

@vanessuniq vanessuniq commented Jan 21, 2025

Summary

This branch updates the existing method resource_is_valid? to conditionally log in messages to runnable to accomodate scenario where the user may not want to log validation messages. The method was updated by adding an additional parameter add_messages_to_runnable with a default value of true, that can be toggle off or on depending on the use case.

…unnable

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>
Signed-off-by: Vanessa Fotso <vfotso@mitre.org>
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.28%. Comparing base (3a23fe1) to head (b07c376).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #595   +/-   ##
=======================================
  Coverage   84.28%   84.28%           
=======================================
  Files         274      274           
  Lines       11601    11603    +2     
  Branches     1291     1295    +4     
=======================================
+ Hits         9778     9780    +2     
  Misses       1815     1815           
  Partials        8        8           
Flag Coverage Δ
backend 92.63% <100.00%> (+<0.01%) ⬆️
frontend 79.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Vanessa Fotso <vfotso@mitre.org>
@@ -157,7 +157,7 @@ def exclude_message(&block)
end

# @see Inferno::DSL::FHIRResourceValidation#resource_is_valid?
def resource_is_valid?(resource, profile_url, runnable)
def resource_is_valid?(resource, profile_url, runnable, log_messages: true) # rubocop:disable Metrics/CyclomaticComplexity
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think log_messages accurately describes this functionality. I think something along the lines of persist_messages, add_messages_to_runnable, or omit_messages would be a more accurate description of the functionality.

@dehall
Copy link
Contributor

dehall commented Jan 22, 2025

I can't add a comment to the line because it wasn't changed, but there's another instance of adding a message to the runnable when an exception occurs:

runnable.add_message('error', e.message)

Should that be conditional too? There's still going to be an exception

@Jammjammjamm
Copy link
Collaborator

Should that be conditional too? There's still going to be an exception

I think it makes sense to keep that as is. If an exception prevents validation, that is always useful information.

@vanessuniq vanessuniq merged commit 843491c into main Jan 22, 2025
10 checks passed
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