fix(feed): Parse MusicAudio and Video posts metadata on Map Feed #1400

sujal-into · 2024-11-20T12:09:54Z

netlify · 2024-11-20T12:10:09Z

✅ Deploy Preview for testitori ready!

Name	Link
🔨 Latest commit	`0b9046c`
🔍 Latest deploy log	https://app.netlify.com/sites/testitori/deploys/6743f6f08217980008ba7882
😎 Deploy Preview	https://deploy-preview-1400--testitori.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2024-11-20T12:10:20Z

✅ Deploy Preview for teritori-dapp ready!

Name	Link
🔨 Latest commit	`1ac1350`
🔍 Latest deploy log	https://app.netlify.com/sites/teritori-dapp/deploys/6748b320d6714e0008774bbe
😎 Deploy Preview	https://deploy-preview-1400--teritori-dapp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

WaDadidou · 2024-11-20T21:02:36Z

packages/components/socialFeed/Map/Map.web.tsx

      const metadata = zodTryParseJSON(
-        ZodSocialFeedPostMetadata,
+        ZodSocialFeedPostMetadata.partial().merge(
+          ZodSocialFeedPostMetadata.pick({ title: true }),


You don't need to modify the Zod type to try parse.
Yes, ZodSocialFeedPostMetadata shape can be wrong, but we have made other shapes for other posts categories. So we need to handle them.

Reformulation: The issue is due to the fact we have 4 differents types for the fetched posts.
Here we only try to parse ZodSocialFeedPostMetadata. So, the tracks and videos posts validation fails.

The result here has no success: https://github.com/TERITORI/teritori-dapp/blob/bugfix/video-audio-map/packages/utils/sanitize.ts#L22
If you console.log the result, you will see what's wrong to validate ZodSocialFeedPostMetadata

To avoid that, because the map gathers all posts categories, we must handle these Zod types. Here is my suggestion:

const videoPostMetadata = zodTryParseJSON( ZodSocialFeedVideoMetadata, post.metadata, ); const trackMetadata = zodTryParseJSON( ZodSocialFeedTrackMetadata, post.metadata, ); const articleMetadata = zodTryParseJSON( ZodSocialFeedArticleMetadata, post.metadata, ); const postMetadata = zodTryParseJSON( ZodSocialFeedPostMetadata, post.metadata, ); const metadataToUse = videoPostMetadata || trackMetadata || articleMetadata || postMetadata;

We need a refacto about it. Either unify all types in one, either add an utilitary function to handle (try parse) these differents zod types

actually no there is the zodSocialFeedCommonMetadata schema when you need to only parse the title

I have made a fix for this discussion in this commit: 3a16113

actually no there is the zodSocialFeedCommonMetadata schema when you need to only parse the title

In this case, there are other things that i need to parse also based on the category type, so I have created a util function to handle those cases.

you can switch on the post type to do the proper parsing in each case, the whole point of using zod is to have type safety, using a z.ZodTypeAny entirely defeat this purpose

I have used switch and removed the ZodTypeAny : ece2bac

n0izn0iz

no z.ZodTypeAny please

WaDadidou · 2024-11-21T16:59:31Z

packages/utils/types/feed.ts

+  switch (category) {
+    case PostCategory.Video:
+      return zodTryParseJSON(ZodSocialFeedVideoMetadata, metadata);
+    case PostCategory.Article:
+      return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata);
+    case PostCategory.MusicAudio:
+      return zodTryParseJSON(ZodSocialFeedTrackMetadata, metadata);
+    default:
+      return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata);
+  }


Suggested change

switch (category) {

case PostCategory.Video:

return zodTryParseJSON(ZodSocialFeedVideoMetadata, metadata);

case PostCategory.Article:

return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata);

case PostCategory.MusicAudio:

return zodTryParseJSON(ZodSocialFeedTrackMetadata, metadata);

default:

return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata);

}

switch (category) {

case PostCategory.Video:

return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata) || zodTryParseJSON(ZodSocialFeedVideoMetadata, metadata);

case PostCategory.Article:

return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata) || zodTryParseJSON(ZodSocialFeedArticleMetadata, metadata);

case PostCategory.MusicAudio:

return zodTryParseJSON(ZodSocialFeedTrackMetadata, metadata);

default:

return zodTryParseJSON(ZodSocialFeedPostMetadata, metadata);

}

Use ZodSocialFeedArticleMetadata for Articles

Hanlde the "old" posts. Example: https://github.com/TERITORI/teritori-dapp/blob/bugfix/video-audio-map/packages/components/socialFeed/Map/MapPosts/ArticleMapPost.tsx#L24-L26

We'll make another PR to replace all zodTryParseJSON usages by parseSocialFeedMetadata (About posts)

Hanlde the "old" posts

actually we will reset the feed after redesigning the contracts so I don't think it's important to support old stuff right now

even "We'll make another PR to replace all zodTryParseJSON usages by parseSocialFeedMetadata (About posts)" is not necessary since we'll have well defined types from contracts

I made changes here: 8a0ebc6

@n0izn0iz Yes, some issues are "stand by" for now

n0izn0iz

if you need other cases, add them, there should be one zod schema for one post category and the fallback is common metadata

packages/utils/types/feed.ts

Co-authored-by: n0izn0iz <[email protected]>

n0izn0iz · 2024-11-23T03:07:47Z

packages/utils/types/feed.ts

+      return (
+        zodTryParseJSON(ZodSocialFeedPostMetadata, metadata) ||
+        zodTryParseJSON(ZodSocialFeedVideoMetadata, metadata)
+      );


Suggested change

return (

zodTryParseJSON(ZodSocialFeedPostMetadata, metadata) ||

zodTryParseJSON(ZodSocialFeedVideoMetadata, metadata)

);

return (

zodTryParseJSON(ZodSocialFeedVideoMetadata, metadata)

);

Resolved it here: 0b9046c

n0izn0iz · 2024-11-23T03:07:59Z

packages/utils/types/feed.ts

+      return (
+        zodTryParseJSON(ZodSocialFeedPostMetadata, metadata) ||
+        zodTryParseJSON(ZodSocialFeedArticleMetadata, metadata)
+      );


Suggested change

return (

zodTryParseJSON(ZodSocialFeedPostMetadata, metadata) ||

zodTryParseJSON(ZodSocialFeedArticleMetadata, metadata)

);

return (

zodTryParseJSON(ZodSocialFeedArticleMetadata, metadata)

);

Resolved it here: 0b9046c

netlify · 2024-11-28T18:15:10Z

✅ Deploy Preview for gno-dapp ready!

Name	Link
🔨 Latest commit	`1ac1350`
🔍 Latest deploy log	https://app.netlify.com/sites/gno-dapp/deploys/6748b3204f07db0008437d39
😎 Deploy Preview	https://deploy-preview-1400--gno-dapp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

WaDadidou · 2024-11-28T18:49:33Z

So, we pause that, we need refacto

fix: made keys for social post optional while parsing except title

f2115fb

sujal-into added the Social Feed Everything related to the Social Feed label Nov 20, 2024

sujal-into requested review from n0izn0iz and WaDadidou November 20, 2024 12:09

sujal-into self-assigned this Nov 20, 2024

WaDadidou requested changes Nov 20, 2024

View reviewed changes

fix: added a function to parse different social feed categories metadata

3a16113

sujal-into requested a review from WaDadidou November 21, 2024 08:05

n0izn0iz requested changes Nov 21, 2024

View reviewed changes

fix: used switch case to parse different social feed metadata

ece2bac

WaDadidou requested changes Nov 21, 2024

View reviewed changes

WaDadidou changed the title ~~fix: made keys for social post optional while parsing except title~~ fix(feed): Parse MusicAudio and Video posts metadata on Map Feed Nov 21, 2024

n0izn0iz requested changes Nov 21, 2024

View reviewed changes

packages/utils/types/feed.ts Outdated Show resolved Hide resolved

packages/utils/types/feed.ts Show resolved Hide resolved

sujal-into and others added 3 commits November 22, 2024 09:42

fix: Changed to feedArticle metadata for article type

7a18deb

Co-authored-by: n0izn0iz <[email protected]>

default social media parse to common metadata

ac6d3b5

Co-authored-by: n0izn0iz <[email protected]>

fix: feed post metadata check for video and article

8a0ebc6

sujal-into requested review from WaDadidou and n0izn0iz November 22, 2024 09:34

n0izn0iz requested changes Nov 23, 2024

View reviewed changes

fix: removed post metdata option from video and article type

0b9046c

sujal-into requested a review from n0izn0iz November 25, 2024 04:04

Merge branch 'main' into bugfix/video-audio-map

1ac1350

WaDadidou enabled auto-merge November 28, 2024 18:15

WaDadidou added the Paused label Nov 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(feed): Parse MusicAudio and Video posts metadata on Map Feed #1400

fix(feed): Parse MusicAudio and Video posts metadata on Map Feed #1400

sujal-into commented Nov 20, 2024

netlify bot commented Nov 20, 2024 •

edited

Loading

netlify bot commented Nov 20, 2024 •

edited

Loading

WaDadidou Nov 20, 2024 •

edited

Loading

n0izn0iz Nov 21, 2024 •

edited

Loading

sujal-into Nov 21, 2024

sujal-into Nov 21, 2024

n0izn0iz Nov 21, 2024

sujal-into Nov 21, 2024 •

edited

Loading

n0izn0iz left a comment

WaDadidou Nov 21, 2024

n0izn0iz Nov 21, 2024 •

edited

Loading

sujal-into Nov 22, 2024

WaDadidou Nov 23, 2024

n0izn0iz left a comment

n0izn0iz Nov 23, 2024

sujal-into Nov 25, 2024

n0izn0iz Nov 23, 2024

sujal-into Nov 25, 2024

netlify bot commented Nov 28, 2024 •

edited

Loading

WaDadidou commented Nov 28, 2024

fix(feed): Parse MusicAudio and Video posts metadata on Map Feed #1400

Are you sure you want to change the base?

fix(feed): Parse MusicAudio and Video posts metadata on Map Feed #1400

Conversation

sujal-into commented Nov 20, 2024

netlify bot commented Nov 20, 2024 • edited Loading

✅ Deploy Preview for testitori ready!

netlify bot commented Nov 20, 2024 • edited Loading

✅ Deploy Preview for teritori-dapp ready!

WaDadidou Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

n0izn0iz Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sujal-into Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

n0izn0iz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n0izn0iz Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n0izn0iz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Nov 28, 2024 • edited Loading

✅ Deploy Preview for gno-dapp ready!

WaDadidou commented Nov 28, 2024

netlify bot commented Nov 20, 2024 •

edited

Loading

netlify bot commented Nov 20, 2024 •

edited

Loading

WaDadidou Nov 20, 2024 •

edited

Loading

n0izn0iz Nov 21, 2024 •

edited

Loading

sujal-into Nov 21, 2024 •

edited

Loading

n0izn0iz Nov 21, 2024 •

edited

Loading

netlify bot commented Nov 28, 2024 •

edited

Loading