Rewrite angular service - input-matcher

[jiji14]

Rewrite input-matcher.js Angular service

Related Issue: GitHub Issue #1001

Input-matcher is a function designed to check if the user's trip input is correct and print logs. I've kept the fundamental logic while updating the old JavaScript syntax and adding input/output types for each function.

1. Replacing moment with Luxon

Previously, the Moment format function was used to print logs. I have replaced moment with Luxon.

Here's a comparison of the two outputs:

2. Unit Testing

I have written test cases for both valid and invalid inputs for each function. To ensure proper testing, please add "testEnvironment": "jsdom" to your jest.config.json (I haven't pushed the update for jest.config.json to avoid conflicts).

Here are the test results:

I had to merge onboarding_routing_sept_2023 branch because service_rewrite_2023 is too out-dated.

[JGreenlee]

resolved merge conflicts

[JGreenlee]

@shankari So what do we want to do with this PR? Are we going to change the tests to match the ones on the server (#1061 (comment))? Are we going to do that now or later?

One other option is to let #1086 absorb this PR since it includes these changes and builds on top of them. We could leave this PR open and come back to it when we figure out the testing strategy.

[shankari]

@JGreenlee thanks for the clarification around TestUserInputFakeData.py and the different tests here.
I think we can go ahead and merge this now, but we should file a separate cleanup issue to pull out the javascript + python input matcher code into a separate module, and unify the tests in both languages.

This wasn't in "Ready for review", so I'm going to go and review #1086 for now just in case there's anything else pending here. But the tests are passing, so if this is moved into the correct column, I am happy to merge ASAP

[shankari]

It is really hard to review this change given the extraneous whitespace.
@jiji14 @JGreenlee can one of you please fix the prettier issues so that I can focus on the actual changes?

[jiji14]

It is really hard to review this change given the extraneous whitespace. @jiji14 @JGreenlee can one of you please fix the prettier issues so that I can focus on the actual changes?

@shankari Let me fix this!

[jiji14]

@shankari

I deleted inputMatcher dependency in factory, and then Prettier changed the line break since it became shorter than the original one. That's why the white space got altered. The only solution I can think of to fix this issue is to use prettier ignore line so that you don't see the changes, but I don't think it's a good idea. What do you think?

[ before ]

  .factory(
    'EnketoNotesButtonService',
    function (InputMatcher, EnketoSurveyAnswer, Logger, $timeout) {
      var enbs = {};

[ after ]

  .factory('EnketoNotesButtonService', function (EnketoSurveyAnswer, Logger, $timeout) {
    var enbs = {};

[JGreenlee]

I deleted inputMatcher dependency in factory, and then Prettier changed the line break since it became shorter than the original one. That's why the white space got altered.

Interesting that Prettier actually changes the indent level based on one line being longer or shorter.

I agree and think you should leave it how it is. When we look at the diff, we can just enable the 'Hide whitespace' option to make it easier to review

[shankari]

@jiji14 if there is a real change in the line, I am OK with associate whitespace changes. I just don't like whitespace changes with no associated code changes caused by different editors.

When we look at the diff, we can just enable the 'Hide whitespace' option to make it easier to review

I will probably not use "Hide whitespace" because it will also hide the spurious whitespace changes. But maybe if Prettier ensures that we never get those...

[shankari]

if there is a real change in the line, I am OK with associate whitespace changes.

Although it is really annoying that because of the difference in indentation, pretty much every single line was changed.
Particularly annoying since I have been very picky about whitespace changes to keep the commit history clean.
I wonder if it would be worthwhile to use prettier ignore when we have such large scale extraneous changes, and periodically remove them in a single commit labeled "prettier catch up" or something.

Let's see how often this happens before deciding...

[shankari]

@JGreenlee I just merged this. If you pull it into #1086 I can review that tomorrow.