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

Manual input test #23

Merged
merged 56 commits into from
Jan 6, 2025
Merged

Manual input test #23

merged 56 commits into from
Jan 6, 2025

Conversation

VoldemLive
Copy link
Collaborator

Manual Input Blood Test Results Feature

Features

  1. Create new blood test with:

    • Biomarker selection
    • Value input
    • Automatic reference range assignment
    • Automatic unit assignment
    • Optional notes
    • Patient selection (for users with full access)
  2. Validation rules:

    • Value must be non-negative
    • Value must be numeric
    • Biomarker must be selected
    • Reference range must exist
    • Unit must be present
  3. UI Features:

    • Dynamic reference range display
    • Error messages display
    • Success messages after creation
    • Form field validation indicators

Test Plan

  1. Unit Tests:

    • Model validations for LabTest
    • Associations between models
    • Value validation (negative, zero, positive, non-numeric)
    • Required field validations
  2. Integration Tests:

    • Successful blood test creation
    • Error handling for invalid inputs
    • Form submission with all required fields
    • JavaScript-dependent functionality
  3. Feature Tests:

    • Complete form submission workflow
    • Dynamic field updates
    • Error message display
    • Success message display
    • Patient selection for authorized users
  4. Edge Cases:

    • Boundary values for reference ranges
    • Special characters in inputs
    • Zero and negative values
    • Missing required fields
    • Invalid biomarker selections

TEST RESULTS:

...Capybara starting Puma...

  • Version 6.4.3, codename: The Eagle of Durango
  • Min threads: 0, max threads: 4
  • Listening on http://127.0.0.1:64570
    .....................

Finished in 5.04 seconds (files took 1.7 seconds to load)
24 examples, 0 failures

@VoldemLive VoldemLive requested a review from aksafan December 14, 2024 23:00
Copy link
Collaborator

@aksafan aksafan left a comment

Choose a reason for hiding this comment

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

Well done with the task and tests!

Please look at comments, especially about "Blood" test naming and SOLID.
Also, please target develop as a branch for all feature requests.

.DS_Store Outdated Show resolved Hide resolved
.rubocop.yml Outdated Show resolved Hide resolved
.rubocop.yml Outdated Show resolved Hide resolved
.rubocop.yml Outdated Show resolved Hide resolved
.rubocop.yml Outdated Show resolved Hide resolved
app/views/lab_tests/new_blood_test.html.erb Outdated Show resolved Hide resolved
app/views/lab_tests/new_blood_test.html.erb Outdated Show resolved Hide resolved
app/views/lab_tests/new_blood_test.html.erb Outdated Show resolved Hide resolved
db/seeds.rb Outdated Show resolved Hide resolved
spec/controllers/lab_tests_controller_spec.rb Outdated Show resolved Hide resolved
@VoldemLive VoldemLive requested a review from aksafan December 26, 2024 22:49
Copy link
Collaborator

@aksafan aksafan left a comment

Choose a reason for hiding this comment

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

Well done in LabTest controller.
Almost ready for being approved.

Please make sure to answer and cover all comments left.

app/controllers/lab_tests_controller.rb Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/policies/lab_test_policy.rb Outdated Show resolved Hide resolved
app/views/lab_tests/new.html.erb Outdated Show resolved Hide resolved
app/views/lab_tests/new.html.erb Show resolved Hide resolved
app/views/lab_tests/new.html.erb Show resolved Hide resolved
config/routes.rb Show resolved Hide resolved
@VoldemLive VoldemLive requested a review from aksafan December 27, 2024 03:37
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/models/lab_test.rb Outdated Show resolved Hide resolved
config/routes.rb Show resolved Hide resolved
spec/models/lab_test_spec.rb Outdated Show resolved Hide resolved
.rubocop.yml Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
app/controllers/biomarkers_controller.rb Outdated Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/controllers/lab_tests_controller.rb Outdated Show resolved Hide resolved
app/views/lab_tests/new.html.erb Outdated Show resolved Hide resolved
app/views/lab_tests/new.html.erb Show resolved Hide resolved
app/views/lab_tests/new.html.erb Show resolved Hide resolved
config/routes.rb Show resolved Hide resolved
app/models/lab_test.rb Outdated Show resolved Hide resolved
@VoldemLive VoldemLive requested a review from aksafan January 3, 2025 21:32
@VoldemLive VoldemLive changed the base branch from main to develop January 3, 2025 21:34
@VoldemLive VoldemLive requested a review from aksafan January 4, 2025 23:10
app/helpers/users_helper.rb Outdated Show resolved Hide resolved
@VoldemLive
Copy link
Collaborator Author

When showing validation errors, there is an inconsistent message for the value field and there is a duplication of error messages on a page: image

removed additional error label

@aksafan
Copy link
Collaborator

aksafan commented Jan 6, 2025

When showing validation errors, there is an inconsistent message for the value field and there is a duplication of error messages on a page: image

removed additional error label

@VoldemLive Well done!

So only set_user issue left.

@VoldemLive VoldemLive requested a review from aksafan January 6, 2025 00:46
@VoldemLive
Copy link
Collaborator Author

When showing validation errors, there is an inconsistent message for the value field and there is a duplication of error messages on a page: image

removed additional error label

@VoldemLive Well done!

So only set_user issue left.

got it and fixed.

@aksafan
Copy link
Collaborator

aksafan commented Jan 6, 2025

When showing validation errors, there is an inconsistent message for the value field and there is a duplication of error messages on a page: image

removed additional error label

@VoldemLive Well done!
So only set_user issue left.

got it and fixed.

@VoldemLive where?
It still in code:
image

The error appears when you haven't set a user properly here.
This happens cause here you have nil in params[:user_id] and you can't call nil.present?.
And this happens cause there is no user_id in params, there is only lab_test in params and user_id is inside that lab_test:
image

So that user_id is nested in lab_test. You need to handle this.

You can just extract it from lab_test_params.

@VoldemLive
Copy link
Collaborator Author

The error appears when you haven't set a user properly here.
This happens cause here you have nil in params[:user_id] and you can't call nil.present?.
And this happens cause there is no user_id in params, there is only lab_test in params and user_id is inside that lab_test:

you are right, rewrited:
def set_user if current_user.full_access_roles_can? User.find(lab_test_params[:user_id]) else current_user end end

in a few sec will push fixes

@aksafan
Copy link
Collaborator

aksafan commented Jan 6, 2025

def set_user if current_user.full_access_roles_can? User.find(lab_test_params[:user_id]) else current_user end end

@VoldemLive Great!

One question: what if there is no user_id in lab_test_params or lab_test_params[:user_id] is empty?
I assume, that check won't work.

What do you think about this?

  def set_user
    if current_user.full_access_roles_can? && lab_test_params[:user_id].present?
      User.find(lab_test_params[:user_id])
    else
      current_user
    end
  end

This will cover both of those cases.

@VoldemLive
Copy link
Collaborator Author

One question: what if there is no user_id in lab_test_params or lab_test_params[:user_id] is empty? I assume, that check won't work.

What do you think about this?

hmm ... we still need to check it in "if" before, or we will crach the app

@aksafan
Copy link
Collaborator

aksafan commented Jan 6, 2025

One question: what if there is no user_id in lab_test_params or lab_test_params[:user_id] is empty? I assume, that check won't work.
What do you think about this?

hmm ... we still need to check it in "if" before, or we will crach the app

Exactly!

@aksafan
Copy link
Collaborator

aksafan commented Jan 6, 2025

@VoldemLive Well done with all of fixes and changes!

Huge gratz on merging your first pull request!
I'm proud of your stamina and agility!

@aksafan aksafan merged commit 618132b into develop Jan 6, 2025
2 checks passed
@aksafan aksafan deleted the manual_input_test branch January 6, 2025 01:32
@VoldemLive
Copy link
Collaborator Author

VoldemLive commented Jan 6, 2025

@VoldemLive Well done with all of fixes and changes!

Huge gratz on merging your first pull request! I'm proud of your stamina and agility!

Thank you Anton for the persistence and patience in helping beginners. I need to drink ))))))

@aksafan aksafan linked an issue Jan 11, 2025 that may be closed by this pull request
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.

Manual input of blood tests
2 participants