Remove code that attempts to fetch location data#1760
Remove code that attempts to fetch location data#1760MartinRiese wants to merge 4 commits intomasterfrom
Conversation
It is not implmented on the web apps side, nor is there a plan for it
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1760 +/- ##
============================================
- Coverage 70.36% 70.36% -0.01%
+ Complexity 2032 2017 -15
============================================
Files 257 255 -2
Lines 7995 7942 -53
Branches 755 754 -1
============================================
- Hits 5626 5588 -38
+ Misses 2087 2073 -14
+ Partials 282 281 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| public EvaluationContext getEvalContextWithHereFuncHandler() { | ||
| EvaluationContext ec = sessionWrapper.getEvaluationContext(); | ||
| ec.addFunctionHandler(new FormplayerHereFunctionHandler(this, currentBrowserLocation)); | ||
| ec.addFunctionHandler(new ScreenUtils.HereDummyFunc(-23.56, -46.66)); |
There was a problem hiding this comment.
Can this method be still called ? If not think we should we hard crash here instead with a IllegalStateException so that we know this code is indeed not being used.
There was a problem hiding this comment.
If we are fine with getting an error when somebody does end up using the here() function, then yes. This could just be removed and all getEvalContextWithHereFuncHandler could be replaced with getSessionWrapper().getEvaluationContext().
There was a problem hiding this comment.
I do think if we are not going to support here, it should result into a very explicit error on Web Apps UI when someone does use it instead of giving an impression that everything is fine.
There was a problem hiding this comment.
Chatted with Woody and he thinks it would be better to throw the exception.
|
There are some web apps in production that use the https://docs.google.com/spreadsheets/d/1nQSnugJBreKLzxCoH8JAVGdG8MC_bL1OKaUDW6-i5YM/edit?usp=sharing |
Product Description
It is not implemented on the web apps side, nor is there a plan for it.
There is some code in web apps that looks for
shouldRequestLocationbut only fornavigate_menurequests. The added logging showed that this has not been called in over a month.For testing I added the
here()function the a case list and case details. For the list theHereDummyFuncwas used already. For the details theget_detailsrequest hadshouldRequestLocationset totruebut was ignored by the web apps code.The documentation states that the
here()function is only implemented for Android apps. So I think it is fairly save to remove the related code from formplayer as well.Technical Summary
https://dimagi.atlassian.net/browse/USH-6280
Safety Assurance
Safety story
Tested locally
Automated test coverage
No
QA Plan
Special deploy instructions
Rollback instructions
Review