refactor(noise-api): Update noise api component

[Zen-cronic]

Fixes #1 and #2 (part of the same component)

Changes:

• Added vitest for testing functions
• Added zod for schema validation

Types and comment descriptions referred from the original Plotly repo.

[MaanasArora]

Thank you @Zen-cronic! I really like how you are thinking about how to make the API more well-tested and robust.

A few comments:

• Do we want the default request method in makeTracketApiRequest to be GET?
  • My rationale was that we might want a single method to handle both GET, POST, etc. requests for maintainability and a single 'source of truth' / to reduce duplication
  • If we do go the route in this PR, is there a need for the two methods makeTracketApiRequest and getTracketApi at all?
• We will probably not want the timezone to be automatically adjusted to a constant because we would like to encourage this application to be used in other cities
• It seems Zod is standard for TypeScript type checking, so thanks for noticing we need more type safety!
  • Is there a way to make creating the API endpoints more accessible to new developers? I'm not fully sure about how to strike the right balance between the complexity of the frontend and its simplicity/ease of use
• I didn't find the noise API requests in this PR if you intended to add them (no worries if not, this is all a lot!)

Thanks again!

[Zen-cronic]

no worries at all!
My overall approach is to mirror the v1 dash repo. So the logic is borrowed from there.

Re:

1. In the v1 repo, GET seems to be the only type of http method used: https://github.com/CivicTechTO/tRacket-dashboard/blob/6a0a3c82ba72210d81b8b0edde61858039cb70f9/app/src/data_loading/noise_api.py#L38. Let me know if you find other methods being called elsewhere, then we should definitely have a single source of truth just like you mentioned.

2. You're right. Having both makeTracketApiRequest and getTracketApi is redundant. I was just starting out with the initial code you wrote, but we can make do with just one of them.

3. Gotcha, we should have a more flexible input for the timezone like how it's handled in the recent commits to the v1 repo: CivicTechTO/tRacket-dashboard@628811f

4. Yep, pydantic from the v1 repo is mirrored as zod. All of the validation/transformation logic are borrowed from that repo as well, e.g., https://github.com/CivicTechTO/tRacket-dashboard/blob/6a0a3c82ba72210d81b8b0edde61858039cb70f9/app/src/data_loading/models.py#L41

5. By the 'noise API', do you mean this? If so, the current makeTracketApiRequest acts as the _get method, getLocations is get_locations, and I'll implement the get_location_noise_data next.

Appreciate your pointers.

[MaanasArora]

@Zen-cronic

That all makes sense! I don't think we necessarily need to mirror the Dash repo, though, as we're redoing the whole thing to improve it; we should do whatever (i) works well for our purposes, (ii) makes it easier for people to contribute.

1. We will eventually extend to multiple methods after implementing some new features (particularly user annotations), so it is perhaps best to have the method generic, so that we can add a postTracketApi method eventually.
2. Addressed above in 1.
3. Yes, that would be great! Feel free to do that here, or in another PR (after removing the timezone logic).
4. Ah, I see. I'm open to any implementation here, even if it doesn't mirror the Dash version. Since we are focused on keeping the implementation simple, I'm wondering if using Zod extensively is necessary, though I can understand more safety is never bad; what do you think?
5. Yes, that's what I thought, that you'll add that method. Just wanted to clarify.

Thanks!

[Zen-cronic]

1. Oh I see. I was under impression that this repo would closely mirror the v1 repo, and i wasn't aware about user annotations. Will implement a generic method instead.
2. Same as 1
3. On it. (Probably better to fix it in another PR)
4. Similar to 1. Since we don't have to mirror v1, I say we make do without zod. I'm a proponent of less code == better code! The types will be based on what the backend API returns.
5. On it.

I don't think we necessarily need to mirror the Dash repo, though, as we're redoing the whole thing to improve it; we should do whatever (i) works well for our purposes, (ii) makes it easier for people to contribute.

Noted. Is there any guideline to what's considered improvement from v1? E.g., fewer dependencies? more http methods (like you mentioned)?

[Zen-cronic]

Todo:

• Implement generic http method
• remove getTracketApi
• Add dynamic timezone Moved to another PR
• Remove zod validation - for simplicity
• Add get_location_noise_data (noise api) function

[MaanasArora]

Thank you @Zen-cronic! Looks great to me.

I'll merge this, and perhaps make some changes myself:

• To be clear, I did not mean that the getTracketApi request was redundant as-is, but rather when the method is set by default to "GET" and there is no way to change it in downstream methods.
• I suspect there's ways to make the TypeScript casting simpler, but I'll look into it!

[Zen-cronic]

To be clear, I did not mean that the getTracketApi request was redundant as-is, but rather when the method is set by default to "GET" and there is no way to change it in downstream methods.

I suspect there's ways to make the TypeScript casting simpler, but I'll look into it!

Feel free to implement those. Appreciate it