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

[FIX]: removing unneeded NFT module and related code #108

Merged
merged 2 commits into from
May 3, 2023

Conversation

bengriffin1
Copy link
Contributor

πŸ“¦ Pull Request

For NFT Checkout, approved clients will call our REST API endpoints directly rather than using the SDK. Removing all references to NFT service from this repo.

βœ… Fixed Issues

🚨 Test instructions

⚠️ Don't forget to add a semver label!

@bengriffin1 bengriffin1 added the minor Increment the minor version when merged label May 2, 2023
Comment on lines -32 to -57
test('Successfully POSTs to the given endpoint and adds JSON header', async () => {
const fetchStub = jest.fn().mockImplementation(() => successRes);
(fetch as any) = fetchStub;

await expect(
post(
'https://example.com/hello/world',
API_KEY,
{ public_address: '0x0123' },
{ 'Content-Type': 'application/json' },
),
).resolves.toBe('hello world');

const fetchArguments = fetchStub.mock.calls[0];
expect(fetchArguments).toEqual([
'https://example.com/hello/world',
{
method: 'POST',
headers: {
'X-Magic-Secret-key': API_KEY,
'Content-Type': 'application/json',
},
body: '{"public_address":"0x0123"}',
},
]);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep this? doesn't look NFT related

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this in a PR for minting, basically needed to add in the secret key header specifically for minting -- here's where it was added: https://github.com/magiclabs/magic-admin-js/pull/107/files#diff-1dafd08f79b2abad5cee6ab033b9a92fd6e01257c287673b6eb533a9c558335aR32

This test is for this change, which is only needed by minting: https://github.com/magiclabs/magic-admin-js/pull/108/files#diff-3ca6a15d564c2c26d46054f3bde8c43d1e5c79865143e55ad08164353ef58320L54

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha!

@bengriffin1 bengriffin1 merged commit 6965f15 into magiclabs:master May 3, 2023
@magiclabsFE
Copy link

πŸš€ PR was released in v1.10.0 πŸš€

@magiclabsFE magiclabsFE added the released This issue/pull request has been released. label May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants