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

DR-3305 fallback for featured item updates post reverse proxy launch #254

Merged
merged 14 commits into from
Dec 5, 2024

Conversation

avertrees
Copy link
Collaborator

Ticket:

This PR does the following:

  • updates num of digitized items in default featured item object rendered in campaign hero
  • refactors implementation of default object to remove environment dependency

Open Questions

How has this been tested? How should a reviewer test this?

Accessibility concerns or updates

Checklist:

  • I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.
  • I have updated the CHANGELOG.md.

Copy link

vercel bot commented Nov 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
digital-collections ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2024 5:39pm

CHANGELOG.md Show resolved Hide resolved
foregroundImageSrc: "/482815.jpg",
uuid: "510d47d9-4f93-a3d9-e040-e00a18064a99",
title: "Watuppa, From water front, Brooklyn",
href: "https://qa-digitalcollections.nypl.org/items/510d47d9-4f93-a3d9-e040-e00a18064a99",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this risk linking to QA if this was in production?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to /items/510d47d9-4f93-a3d9-e040-e00a18064a99 to be relative to the environment its in.

href: "https://digitalcollections.nypl.org/items/510d47d9-4f93-a3d9-e040-e00a18064a99",
},
numberOfDigitizedItems: "875,861",
const defaultFeaturedItem: FeaturedItemDataType = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test fails because this was updated but the reference in campaignHero.tsx was not. In https://github.com/NYPL/digital-collections/blob/main/app/src/components/featuredItem/campaignHero.tsx#L13 you can remove the [appConfig["environment"] as ENV_KEY] and also delete the unused imports.

Because you're importing the object from another file, you can make a copy by doing const defaultFeaturedItemResponse = { ...defaultFeaturedItem };.

You're not modifying defaultFeaturedItem in campaignHero.tsx so it might be safe to just use it directly in the useState, but I think it's better practice to make a copy to make it immutable.

@avertrees avertrees merged commit 28849a7 into main Dec 5, 2024
4 checks passed
@avertrees avertrees deleted the feature/DR-3305/update-default-featured-item-data branch December 5, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants