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

Metrics don't appear on production (but fine in storybook) #376

Closed
underbluewaters opened this issue Nov 24, 2024 · 7 comments
Closed

Metrics don't appear on production (but fine in storybook) #376

underbluewaters opened this issue Nov 24, 2024 · 7 comments
Milestone

Comments

@underbluewaters
Copy link
Collaborator

It seems that the templates copied in v7 when running create:report contain a bug that only shows up in production.
Screenshot 2024-11-24 at 3 05 25 PM

metricsWithSketchId expects a single stringified id or an array of strings, but it appears the id is an integer. This isn't a problem when working locally and my test cases all appeared correct in storybook, but then when I went to use the reports after deployment all my results tables were empty where I had used metrics.

@twelch
Copy link
Contributor

twelch commented Nov 25, 2024

Thanks Chad. Can you give me an example of steps to reproduce? Or a report you have I can test in production? When you say “the id” is an integer in production, do you mean the sketchid? Is there some behavior of seaetch you can tell me a little more about? Thanks

I’m curious why I or others haven’t hit this. I don’t think I’ve changed this helper code in some time.

@twelch
Copy link
Contributor

twelch commented Nov 25, 2024

Do you have a fix you made in a project repo you can point me at that addresses it?

@underbluewaters
Copy link
Collaborator Author

Yes the sketchId. I highlighted the section of code where I'm seeing this. It should be reproducable with any new report that uses the create:report command and specifies a datasource, so all my geoprocessing functions ended up with it.

I solved the problem by just specifying [id.toString()] as seen in this function.

@twelch
Copy link
Contributor

twelch commented Dec 13, 2024

@underbluewaters the core issue here seems to be that SeaSketch is sending an integer sketchId, and the framework assumes they will always be a string (sketch and collection level).

export type SketchProperties = Record<string, any> & {
  /** Unique sketch ID */
  id: string;

image

I also note that the underlying childProperties for the collection has the child sketchIds sent as a string. I would guess SeaSketch might be essentially calling toString().

image

I'm not sure whether:

  • the framework should start supporting string | number and we should get consistent. This would be a breaking change, not sure the full implications. This might just shift where coercion happens

  • I can just call toString() at the point of receiving and setting report context and call it a day.

  • Or maybe you are already doing string conversion in SeaSketch in some places but not all and can add one more place?

  • Open to thoughts. I'm leaning towards just coercing in App.tsx on the report client side and call it a day.

As for why this error has not shown up previously. Because the template just switched over to using the sketch properties passed by SeaSketch via iframe message. It didn't before because the childProperties was missing for collections. So instead sketch properties were being passed back by the geoprocessing function to the report client. The geoprocessing function gets them from the sketch geojson service (which might be coercing ID's to string?)

@twelch
Copy link
Contributor

twelch commented Dec 13, 2024

I would guess that SeaSketch legacy used to send strings for sketchID also

@twelch
Copy link
Contributor

twelch commented Dec 13, 2024

I think I will just go ahead coerce it to a string and if something else gets implemented or agreed upon, then it can removed at some point.

@twelch
Copy link
Contributor

twelch commented Dec 30, 2024

Should be resolved in 7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants