Conversation
|
|
Adding a screenshot of a test metric created for joy and whimsy |
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Couple of changes. Think something happened with git. Try to merge with main and resubmit. Most comments mimic those in the other PR.
There was a problem hiding this comment.
The calendar changes seem to come from your previous ticket- probably due to an early fork. Can you discard all non-ticket related changes?
There was a problem hiding this comment.
Same as other comment on calendar changes.
There was a problem hiding this comment.
You can remove it from here during the final commit btw. This was mainly to test + a useful reference for whoever does the frontend later on how the hook works.
src/data/hooks/useSubmitMetrics.ts
Outdated
| IDToken: await auth.currentUser?.getIdToken(), | ||
| ...requestData, | ||
| }); | ||
| /* eslint-disable max-len */ |
There was a problem hiding this comment.
Try not to disable the linter. If the line's too long (doesn't really look like it is), just break it up
src/data/hooks/useSubmitMetrics.ts
Outdated
| return state; | ||
| } | ||
|
|
||
| function validateMetricData(data: unknown): boolean { |
There was a problem hiding this comment.
Make these utils in another file pretty much identical to the cloud function. Also apply the fixes (in professor validation etc.) mentioned there here.
| } | ||
| >; | ||
|
|
||
| export interface MetricTarget { |
There was a problem hiding this comment.
Add types pretty much identical to the ones in firebase-conf, you can just copy-paste
There was a problem hiding this comment.
Not gonna comment on this as its temporary. Add a comment saying to delete later.
| const submitOperation = new Cancellable(); | ||
|
|
||
| async function submit(): Promise<void> { | ||
| setState({ |
There was a problem hiding this comment.
for all these set states try to use the original state definition wherever possible so its clear. Mark the types as they are defined like LoadingStateLoaded<string>
idke64
force-pushed
the
andy/355-submitMetrics-hook
branch
from
c0c59b1 to
da121a9
Compare
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Perfect! TYSM. Will merge in after firebase-conf is merged.
2 participants
Summary
Resolves #355
Adds submitMetrics hook, validation logic, and a sandbox page
#/sandbox/submitMetricsto test the hook. submitMetrics calls the firebase cloud function submitMetrics and retries up to 3 times using exponential backoff.The sandbox page is located in
/src/sandbox/submitMetricsand its route is set up in the router component. These should not go into production.Checklist
useSubmitMetricshook insrc/data/hooksthat given a metrics type (that you will define similar to the previous ticket) will call the firebase function you just made./sandbox/submitMetricswith a sample form that lets you submit metrics using the hook. This should only be available on a dev environment not a prod environment. We will delete this later as it is just for testing, so do not worry about styling at all.How to Test
Testing can be done by going to
#/sandbox/submitMetrics. The user must be authenticated and the firebase endpoint must have the submitMetrics function for the entire process to work. However, testing for errors (invalid data, unauthenticated user) can be done completely locally.