Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Commit

Permalink
Fix bug that was causing duplicate keys in sessions created FromPrope…
Browse files Browse the repository at this point in the history
…rtyArray, when returnUserData = true

Refactor fromPropertyArray

Remove console

Cast to any
  • Loading branch information
lizkenyon committed Apr 1, 2024
1 parent 7390cd1 commit 96a0aab
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 102 deletions.
5 changes: 5 additions & 0 deletions .changeset/curly-pears-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@shopify/shopify-api": patch
---

Fix bug that was causing duplicate keys in Session when using FromPropertyArray with returnUserData = true
49 changes: 49 additions & 0 deletions packages/shopify-api/lib/session/__tests__/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ describe('toPropertyArray and fromPropertyArray', () => {
session.toPropertyArray(test.returnUserData),
test.returnUserData,
);

expect(session.id).toStrictEqual(sessionCopy.id);
expect(session.shop).toStrictEqual(sessionCopy.shop);
expect(session.state).toStrictEqual(sessionCopy.state);
Expand All @@ -466,6 +467,7 @@ describe('toPropertyArray and fromPropertyArray', () => {
expect(session.onlineAccessInfo?.associated_user.id).toStrictEqual(
sessionCopy.onlineAccessInfo?.associated_user.id,
);

if (test.returnUserData) {
expect(
session.onlineAccessInfo?.associated_user.first_name,
Expand Down Expand Up @@ -498,7 +500,54 @@ describe('toPropertyArray and fromPropertyArray', () => {
).toStrictEqual(
sessionCopy.onlineAccessInfo?.associated_user.collaborator,
);
// Test that the user information is correctly moved to the associated_user object from property array
expect(sessionCopy).toEqual(
expect.not.objectContaining({
firstName: session.onlineAccessInfo?.associated_user.first_name,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
lastName: session.onlineAccessInfo?.associated_user.last_name,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
email: session.onlineAccessInfo?.associated_user.email,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
locale: session.onlineAccessInfo?.associated_user.locale,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
emailVerified:
session.onlineAccessInfo?.associated_user.email_verified,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
accountOwner:
session.onlineAccessInfo?.associated_user.account_owner,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
collaborator:
session.onlineAccessInfo?.associated_user.collaborator,
}),
);
expect(sessionCopy).toEqual(
expect.not.objectContaining({
associated_user: {id: session.onlineAccessInfo?.associated_user.id},
}),
);
} else {
expect(sessionCopy.onlineAccessInfo?.associated_user?.id).toStrictEqual(
session.onlineAccessInfo?.associated_user?.id,
);
expect(
sessionCopy.onlineAccessInfo?.associated_user.first_name,
).toBeUndefined();
Expand Down
181 changes: 79 additions & 102 deletions packages/shopify-api/lib/session/session.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-fallthrough */
import {InvalidSession} from '../error';
import {OnlineAccessInfo, OnlineAccessUser} from '../auth/oauth/types';
import {OnlineAccessInfo} from '../auth/oauth/types';
import {AuthScopes} from '../auth/scopes';

import {SessionParams} from './types';
Expand All @@ -16,9 +16,6 @@ const propertiesToSave = [
'onlineAccessInfo',
];

interface AssociatedUserObject {
associated_user: Partial<OnlineAccessUser>;
}
/**
* Stores App information from logged in merchants so they can make authenticated requests to the Admin API.
*/
Expand All @@ -33,9 +30,6 @@ export class Session {
);
}

const associatedUserObj: AssociatedUserObject = {
associated_user: {},
};
const obj = Object.fromEntries(
entries
.filter(([_key, value]) => value !== null && value !== undefined)
Expand All @@ -61,103 +55,86 @@ export class Session {
default:
return [key.toLowerCase(), value];
}
})
}),
);

// Sanitize values
.map(([key, value]) => {
switch (key) {
case 'isOnline':
if (typeof value === 'string') {
return [key, value.toString().toLowerCase() === 'true'];
} else if (typeof value === 'number') {
return [key, Boolean(value)];
}
return [key, value];
case 'scope':
return [key, value.toString()];
case 'expires':
return [key, value ? new Date(Number(value)) : undefined];
case 'onlineAccessInfo':
return [
key,
{
associated_user: {
id: Number(value),
},
},
];
case 'userId':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.id = Number(value)),
];
}
case 'firstName':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.first_name =
String(value)),
];
}
case 'lastName':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.last_name = String(value)),
];
}
case 'email':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.email = String(value)),
];
}
case 'accountOwner':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.account_owner =
Boolean(value)),
];
}
case 'locale':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.locale = String(value)),
];
}
case 'collaborator':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.collaborator =
Boolean(value)),
];
}
case 'emailVerified':
if (returnUserData) {
return [
key,
(associatedUserObj.associated_user.email_verified =
Boolean(value)),
];
}
// If returnUserData is false, return any user keys as passed in
default:
return [key, value];
const sessionData = {
onlineAccessInfo: {
associated_user: {},
},
} as any;
Object.entries(obj).forEach(([key, value]) => {
switch (key) {
case 'isOnline':
if (typeof value === 'string') {
sessionData[key] = value.toString().toLowerCase() === 'true';
} else if (typeof value === 'number') {
sessionData[key] = Boolean(value);
} else {
sessionData[key] = value;
}
}),
) as any;
// If the onlineAccessInfo is not present, we are using the new session info and add it to the object
if (returnUserData) {
obj.onlineAccessInfo = associatedUserObj;
}
Object.setPrototypeOf(obj, Session.prototype);
return obj;
break;
case 'scope':
sessionData[key] = value.toString();
break;
case 'expires':
sessionData[key] = value ? new Date(Number(value)) : undefined;
break;
case 'onlineAccessInfo':
sessionData.onlineAccessInfo.associated_user.id = Number(value);
break;
case 'userId':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.id = Number(value);
break;
}
case 'firstName':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.first_name =
String(value);
break;
}
case 'lastName':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.last_name =
String(value);
break;
}
case 'email':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.email = String(value);
break;
}
case 'accountOwner':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.account_owner =
Boolean(value);
break;
}
case 'locale':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.locale = String(value);
break;
}
case 'collaborator':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.collaborator =
Boolean(value);
break;
}
case 'emailVerified':
if (returnUserData) {
sessionData.onlineAccessInfo.associated_user.email_verified =
Boolean(value);
break;
}
// Return any user keys as passed in
default:
sessionData[key] = value;
}
});
Object.setPrototypeOf(sessionData, Session.prototype);
return sessionData as Session;
}

/**
Expand Down

0 comments on commit 96a0aab

Please sign in to comment.