Fix/homepage images on private repos #1461

alexanderleegs · 2023-08-30T02:54:07Z

Problem

This PR fixes an issue where images on the homepage for private/GGS repos were not showing up as intended. The reason for this issue was the preview components using the outdated v1 endpoint to fetch images - they have been modified to use the updated hook calling the v2 endpoint instead, since the media url is handled by our backend in v2 instead of being offloaded to the frontend.

Resolves IS-527

Test

Go to edithomepage for any private repo (e.g. a-test-v4)
Verify that images show up on
- Hero banner
- Left infopic
- Right infopic
Verify that nested images work as well (e.g. /images/album/nested.jpg)
When creating a new infopic section, no error toast should appear

…ated v2 endpoint

seaerchin

same pattern is repeatedly seen - inclined to move to a custom hook for this or, if useGetMediaHook is only used here, to update the hook.

if renaming data to loadedImageURL works, then that's ok too

seaerchin · 2023-08-30T04:16:36Z

src/templates/homepage/HeroSection.jsx

+  useEffect(() => {
+    if (mediaData) {
+      setLoadedImageURL(mediaData.mediaUrl)
    }
-  )
+  }, [mediaData])


why can't we just rename data to loadedImageURL here?

data consists of a lot of image related data, not just the url!

seaerchin · 2023-08-30T04:17:54Z

src/utils/images.ts

+  const cleanImagePath = decodeURI(imageLink).replace(/^\//, "")
+  const filePathArr = cleanImagePath.split("/")
+  const filePath = filePathArr[filePathArr.length - 1]
+  return filePath


won't this return the last item in the array post split? ie, "/images/album/picture.jpg" -> "picture.jpg"

Good catch, i only checked this on first level images - have made a fix in 62d436c

src/utils/images.ts

Co-authored-by: Qilu Xie <[email protected]>

alexanderleegs · 2023-08-30T08:49:55Z

same pattern is repeatedly seen - inclined to move to a custom hook for this or, if useGetMediaHook is only used here, to update the hook.

if renaming data to loadedImageURL works, then that's ok too

Shifted out in 5cb437a

* docs(editable): add docs for drag drop stuf (#1445) Co-authored-by: seaerchin <[email protected]> * fix(editable): shift out contxt to allow nested dnd (#1448) Co-authored-by: seaerchin <[email protected]> * feat: introduce new version of NotFoundPage (#1435) * Add submarine image for NotFoundPage * Add new version of the NotFoundPage * Add storybook stories for new NotFoundPage * Fix unit test for RouteSelector * fix(ui): changes based on design feedback * IS-322 Setup GrowthBook for FE (#1449) * feat: add growthbook as dep * feat: add growthbook to app * feat: inject user attributes * feat: add default dev mode to be false * feat: inject site name as attribute * feat: add types and featur flag list * chore: minor fixes * feat: add type and conditional siteName attribute * fix: enable dev mode only for local dev * chore: remove duplicate comment * IS-417 - Fixing Selector Issues in E2E tests (#1453) * feat: update text assertion * fix: resources text assertion * fix: method to remove other collaborators * fix: uncomment code * feat: extract constant * feat: swap removeFirstCollaborator with removeOtherCollaborators * fix: use removeOtherCollaborators * chore: remove unused import and function * chore(edithomepage): shift constants out (#1416) * refactor(edithomepage): use new constants * refactor(edithomepage): shift to new folder * refactor(edithomepage): shift preview out into own section --------- Co-authored-by: seaerchin <[email protected]> * IS-508: Introduce Interface for GrowthBook (#1455) * feat: introduce interface, remove constants file * fix: comment out sample flags * ref(contactus): stylings for contact us page editor (#1458) * Enhance AddSectionButton to allow adjusting button text * Add various sections of contact us page * Adjust Editable to allow new droppable zones * Update validators to match Figma messages * Introduce feature-flagged contact us page * Remove unneeded comment * Switch to use editable contexts instead and use modal * Fix validation condition * Fix name for FeatureFlaggedContactUs * Remove some unnecessary casts * Fix RouteSelector unit test * chore: set pointerEvents to none for draggable icon button (#1456) * Feat/is 443 nav bar styling (#1457) * chore: update editable types to allow nav * feat: add pageMenuBody * feat: add resourceMenuBody * feat: add folderMenuBody * feat: add GroupMenuBody * feat: swap new components into editNavBar * chore: fix text for components * Fix: addSectionButton button text prop * chore: pass props to button instead of box * chore: add feature flag for new nav bar (#1459) * chore: add feature flag for new nav bar * fix: tests * feat(feature-flag): add feature flag from gb (#1462) * feat(feature-flag): add feature flag from gb * refactor(navbar): use gb for ff * refactor(feature-flag): add types --------- Co-authored-by: seaerchin <[email protected]> * fix(edithomepage): remove optional tags (#1450) Co-authored-by: seaerchin <[email protected]> * Feat/is 494 smooth scroll (#1439) * refactor(edithomepage): smooth scroll * fix: only scroll after first load * chore: fix rebase errors * Revert "fix: only scroll after first load" This reverts commit 158a4df. * Revert "refactor(edithomepage): smooth scroll" This reverts commit e555c2f. * fix: firstLoad condition * chore: move first load check to separate method * fix: smooth scroll --------- Co-authored-by: seaerchin <[email protected]> Co-authored-by: Alexander Lee <[email protected]> * Fix/homepage images on private repos (#1461) * chore: add util method to retrieve image path * fix: replace image retrieval in homepage template components with updated v2 endpoint * fix: change return of util method to handle nested * Update src/utils/images.ts Co-authored-by: Qilu Xie <[email protected]> * Update src/utils/images.ts Co-authored-by: Qilu Xie <[email protected]> * chore: fix styling * refactor: move image fetching to separate hook --------- Co-authored-by: Qilu Xie <[email protected]> * 0.41.0 --------- Co-authored-by: seaerchin <[email protected]> Co-authored-by: seaerchin <[email protected]> Co-authored-by: Harish <[email protected]> Co-authored-by: Hsu Zhong Jun <[email protected]> Co-authored-by: Alexander Lee <[email protected]> Co-authored-by: Qilu Xie <[email protected]>

alexanderleegs added 2 commits August 30, 2023 10:49

chore: add util method to retrieve image path

eb26063

fix: replace image retrieval in homepage template components with upd…

6ebb78d

…ated v2 endpoint

alexanderleegs temporarily deployed to staging August 30, 2023 03:04 — with GitHub Actions Inactive

alexanderleegs temporarily deployed to staging August 30, 2023 03:05 — with GitHub Actions Inactive

alexanderleegs requested a review from a team August 30, 2023 04:06

seaerchin reviewed Aug 30, 2023

View reviewed changes

fix: change return of util method to handle nested

62d436c

alexanderleegs temporarily deployed to staging August 30, 2023 06:15 — with GitHub Actions Inactive

alexanderleegs temporarily deployed to staging August 30, 2023 06:28 — with GitHub Actions Inactive

QiluXie reviewed Aug 30, 2023

View reviewed changes

src/utils/images.ts Outdated Show resolved Hide resolved

src/utils/images.ts Outdated Show resolved Hide resolved

Update src/utils/images.ts

8db4dc6

Co-authored-by: Qilu Xie <[email protected]>

alexanderleegs temporarily deployed to staging August 30, 2023 07:25 — with GitHub Actions Inactive

Update src/utils/images.ts

112304a

Co-authored-by: Qilu Xie <[email protected]>

alexanderleegs temporarily deployed to staging August 30, 2023 07:26 — with GitHub Actions Inactive

chore: fix styling

ac625de

alexanderleegs temporarily deployed to staging August 30, 2023 08:22 — with GitHub Actions Inactive

alexanderleegs temporarily deployed to staging August 30, 2023 08:37 — with GitHub Actions Inactive

refactor: move image fetching to separate hook

5cb437a

alexanderleegs temporarily deployed to staging August 30, 2023 08:39 — with GitHub Actions Inactive

alexanderleegs requested review from seaerchin and QiluXie August 30, 2023 08:49

seaerchin approved these changes Aug 30, 2023

View reviewed changes

alexanderleegs temporarily deployed to staging August 30, 2023 08:53 — with GitHub Actions Inactive

harishv7 approved these changes Aug 30, 2023

View reviewed changes

alexanderleegs merged commit 67f234f into develop Aug 30, 2023

mergify bot deleted the fix/homepage-images-on-private-repos branch August 30, 2023 08:58

This was referenced Aug 30, 2023

release(0.41.0): merge to develop #1463

Merged

release(0.41.0): merge to prod #1464

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/homepage images on private repos #1461

Fix/homepage images on private repos #1461

alexanderleegs commented Aug 30, 2023 •

edited

Loading

seaerchin left a comment

seaerchin Aug 30, 2023

alexanderleegs Aug 30, 2023

seaerchin Aug 30, 2023

alexanderleegs Aug 30, 2023

alexanderleegs commented Aug 30, 2023

Fix/homepage images on private repos #1461

Fix/homepage images on private repos #1461

Conversation

alexanderleegs commented Aug 30, 2023 • edited Loading

Problem

Test

seaerchin left a comment

Choose a reason for hiding this comment

seaerchin Aug 30, 2023

Choose a reason for hiding this comment

alexanderleegs Aug 30, 2023

Choose a reason for hiding this comment

seaerchin Aug 30, 2023

Choose a reason for hiding this comment

alexanderleegs Aug 30, 2023

Choose a reason for hiding this comment

alexanderleegs commented Aug 30, 2023

alexanderleegs commented Aug 30, 2023 •

edited

Loading