Skip to content

feat: project initial setup#2

Merged
BC-krasnoshapka merged 1 commit intomainfrom
vercel-init
May 31, 2023
Merged

feat: project initial setup#2
BC-krasnoshapka merged 1 commit intomainfrom
vercel-init

Conversation

@bc-alexsaiannyi
Copy link
Copy Markdown
Collaborator

@bc-alexsaiannyi bc-alexsaiannyi commented May 22, 2023

What/Why

Added initial setup to the project and connectors between Vercel and BigCommerce.

This PR covers:

  • home page,
  • product page,
  • collections,
  • search, sorting

Cart functionality will be added in the following PR separetly.

Proof

Tested locally. Video's attached below.

Screen.Recording.2023-05-29.at.12.02.11.mov

@bc-alexsaiannyi bc-alexsaiannyi force-pushed the vercel-init branch 2 times, most recently from e81254f to 743dbaa Compare May 23, 2023 11:56
@bc-alexsaiannyi bc-alexsaiannyi marked this pull request as ready for review May 23, 2023 13:01
@bc-alexsaiannyi bc-alexsaiannyi force-pushed the vercel-init branch 9 times, most recently from f03a404 to 110934b Compare May 25, 2023 13:44
@becomevocal
Copy link
Copy Markdown

Verified the latest works for the core products and search functionality:
Screenshot 2023-05-26 at 11 38 57 AM
Screenshot 2023-05-26 at 11 39 12 AM
Screenshot 2023-05-26 at 11 39 18 AM

The add to cart functionality is error'ing out because of cartId and variantId missing. That can be resolved in the next PR.

Copy link
Copy Markdown

@BC-krasnoshapka BC-krasnoshapka left a comment

Choose a reason for hiding this comment

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

Overall looks good for now, left comments for future improvements.

Comment thread lib/bigcommerce/index.ts
lines.map(async ({ merchandiseId }) => {
const productId = Number(merchandiseId);

const resp = await bigcommerceFetch<BigCommerceProductOperation>({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not critical for now, but maybe we can avoid multiple queries and fetch all products via single query instead, which should be better and faster?

Comment thread lib/bigcommerce/index.ts
};
};

export async function createCart(): Promise<VercelCart> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this function really return promise?

Comment thread lib/bigcommerce/index.ts
for (let removals = lineIds.length; removals > 0; removals--) {
const lineId = lineIds[removals - 1]!;

const res = await bigcommerceFetch<BigCommerceDeleteCartItemOperation>({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also not critical, but maybe we can avoid mutliple mutations and use single instead?

Comment thread lib/bigcommerce/index.ts

const collections = await Promise.all(
collectionIdList.map(async (entityId) => {
const res = await bigcommerceFetch<BigCommerceCollectionOperation>({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a way to fetch all categories via single query?

Comment thread lib/bigcommerce/index.ts
}

export async function getMenu(handle: string): Promise<VercelMenu[]> {
const expectedMenyType = 'footerOrHeader';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expectedMenuType ?

Comment thread lib/bigcommerce/index.ts
return [];
}

// TODO: replace with BC API next Page(s) Methods
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this TODO already implemented?


export const fetchStorefrontToken = async () => {
const response = await fetch(
`${BIGCOMMERCE_API_URL}/stores/${process.env.BIGCOMMERCE_STORE_HASH}/v3/storefront/api-token-customer-impersonation`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also not critical, but we'll need to figure out a way how to not generate new token for every incoming request, which doesn't make sense. cc @zvuki

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we might be considering the approach of just generating a CIT and letting the merchant rotate it before (recommendation) or after it expires. That removes some complex logic with env vars we had in place before.


export default async function Navbar() {
const menu = await getMenu('next-js-frontend-header-menu');
const demoMenu = menu.slice(0, 4);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🍹 Let's throw this logic inside getMenu, that way our view logic doesn't need to know about any "business" logic.


export const fetchStorefrontToken = async () => {
const response = await fetch(
`${BIGCOMMERCE_API_URL}/stores/${process.env.BIGCOMMERCE_STORE_HASH}/v3/storefront/api-token-customer-impersonation`,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we might be considering the approach of just generating a CIT and letting the merchant rotate it before (recommendation) or after it expires. That removes some complex logic with env vars we had in place before.

@BC-krasnoshapka BC-krasnoshapka merged commit b17652b into main May 31, 2023
@BC-krasnoshapka
Copy link
Copy Markdown

Merged as we need this prototype to be handed over to Vercel. @bc-alexsaiannyi will work on comments/suggestions in next PR.

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.

4 participants