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

App crash when due date not saved for encounter #1430

Closed
mahalakshme opened this issue Jun 13, 2024 · 8 comments
Closed

App crash when due date not saved for encounter #1430

mahalakshme opened this issue Jun 13, 2024 · 8 comments
Assignees

Comments

@mahalakshme
Copy link
Contributor

mahalakshme commented Jun 13, 2024

https://avni.freshdesk.com/a/tickets/4174

Analysis:

Currently for scheduled encounters(general or program) earliest_visit_date_time and max_visit_date_time can be null. There are no db constraints and this can happen when visit schedule rule is incorrectly(shelter - prerelease - visit schedule rule of KAP-Survey Enrolment form) written. This was not an issue until 7.3 but with adding of checks for encounters(to see if on edit visits scheduled is same as existing one) leads to app crash because of not sufficient checks added to handle when dates don't exist.

Steps to reproduce:

prod, shelter
assign yourself the "KMC_Kalamba Filter House" catchment, turn off the internet and search for '0082' household subject_type, and try to complete 'Electricity' general encounter.

AC:

  • for the cases like above it should not fail for both general and program encounter
  • if visits are scheduled without earliestVisitDateTime, record a rule failure and report via rule failure telemetry
  • Add null checks in datesAreSame function to handle for the existing encounters saved without datetime fields.

Long term fix: (not part of AC)

Such saving needs to be probably blocked from CSV, visit schedule rule and by adding deferred constraint in db (can be dealt in later releases)

@mahalakshme mahalakshme converted this from a draft issue Jun 13, 2024
@mahalakshme mahalakshme moved this from In Analysis to Ready in Avni Product Jun 13, 2024
@mahalakshme mahalakshme changed the title Add check surrounding datesAreSame App crash when due date not saved for encounter Jun 13, 2024
@1t5j0y 1t5j0y moved this from Ready to In Progress in Avni Product Jun 14, 2024
@1t5j0y 1t5j0y self-assigned this Jun 14, 2024
1t5j0y added a commit that referenced this issue Jun 17, 2024
…sitDateTime

Return empty array instead of pre-existing visits if error occurs during visit schedule rule execution
Check for null before converting toISTDate
@1t5j0y
Copy link
Contributor

1t5j0y commented Jun 17, 2024

AC updated in issue description after discussion of how this should be handled.

@1t5j0y 1t5j0y moved this from In Progress to Code Review Ready in Avni Product Jun 17, 2024
@mahalakshme
Copy link
Contributor Author

mahalakshme commented Jun 17, 2024

@1t5j0y so you mean app will also not get crashed like now (say for the above case when saving electricity encounter) right? asking since app is not getting crashed currently when scheduling visits. Its happening when comparing datesAreSame with already scheduled visits which doesn't have datetime.

@1t5j0y
Copy link
Contributor

1t5j0y commented Jun 17, 2024

correct. Visit schedule rules returning visits without earliestVisitDateTime will fail and be reported to rule failure telemetry and not be scheduled so will not crash since they will not be checked via datesAreSame. A null check has been added to toISTDate (called by datesAreSame) where the original crash was happening just in case as well.

@mahalakshme
Copy link
Contributor Author

@1t5j0y okay got it. the null checks will help for the existing encounters already scheduled wrongly and the rule failure will help for the future ones. so have appended the previous AC to the newly added ones.

@1t5j0y 1t5j0y moved this from Code Review Ready to In Progress in Avni Product Jun 17, 2024
1t5j0y added a commit that referenced this issue Jun 17, 2024
…since the pre-existing visits are returned by default
@1t5j0y 1t5j0y moved this from In Progress to Code Review Ready in Avni Product Jun 17, 2024
@petmongrels petmongrels moved this from Code Review Ready to In Code Review in Avni Product Jun 18, 2024
@petmongrels
Copy link
Contributor

for QA

  • rule editing is required for testing this
  • we can test against the data we have for shelter in pre-release
  • from rule we have to return null dates in different fields to test this

@petmongrels petmongrels moved this from In Code Review to QA Ready in Avni Product Jun 18, 2024
@AchalaBelokar AchalaBelokar moved this from QA Ready to In QA in Avni Product Jun 18, 2024
@AchalaBelokar
Copy link

AchalaBelokar commented Jun 24, 2024

  • I Login with dinesh@shelter I register the household after registration visits are not schedule it is happening on prerelease ..

Extract note :

The issue is encounter not scheduled in the latest version 9.0

env : pre-release
org : ruletest
user : dinesh@ruletest
version 9.0

My.Movie.mp4

Note : this is not happening in 8.1

env : pre-release
org : ruletest
user : dinesh@ruletest
version : 8.1

My.Movie.mp4

@AchalaBelokar AchalaBelokar moved this from In QA to QA Failed in Avni Product Jun 24, 2024
@1t5j0y 1t5j0y moved this from QA Failed to In Progress in Avni Product Jun 24, 2024
1t5j0y added a commit that referenced this issue Jun 24, 2024
@1t5j0y 1t5j0y moved this from In Progress to Code Review Ready in Avni Product Jun 24, 2024
@petmongrels petmongrels moved this from Code Review Ready to Code Review with Comments in Avni Product Jun 24, 2024
@petmongrels petmongrels moved this from Code Review with Comments to In Code Review in Avni Product Jun 24, 2024
@petmongrels petmongrels moved this from In Code Review to QA Ready in Avni Product Jun 24, 2024
@AchalaBelokar AchalaBelokar moved this from QA Ready to In QA in Avni Product Jun 25, 2024
@Dinesh2019
Copy link

Scenario 1 : Unplanned visit

if the visit is done as unplanned visit they the visit getting scheduled with earliest visit as null and it is not creating any rule failure

Screenshot of the db:

Image

screenshot of cilent app :

Image

Scenario 2 : scheduled the encounter with earliestvisit date as Null

if complete the registration and the general encounter needs to scheduled but if the earliest visit date is null it is not scheduling the visit. And rule failure is capture as expected

screenshot of webapp :

Image

screenshot of cilent app :

Image

@AchalaBelokar AchalaBelokar moved this from In QA to QA Failed in Avni Product Jun 25, 2024
@1t5j0y
Copy link
Contributor

1t5j0y commented Jun 26, 2024

@Dinesh2019 @AchalaBelokar scenario 1 is working as expected since we performed an unscheduled visit in that case and did not schedule a visit. For an unscheduled visit, earliestVisitDateTime will be null.

@1t5j0y 1t5j0y moved this from QA Failed to QA Ready in Avni Product Jun 26, 2024
@AchalaBelokar AchalaBelokar moved this from QA Ready to In QA in Avni Product Jun 27, 2024
@AchalaBelokar AchalaBelokar moved this from In QA to Done in Avni Product Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

6 participants