-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor: Turn off whitelist mode #123
Conversation
WalkthroughThe updates across various TypeScript files suggest a shift in the application's functionality, particularly around campaign management and NFT minting. Form elements related to markdown and whitelist user checks have been removed or disabled, and new logic has been introduced to handle NFT minting with a focus on collection names. The payment amount for minting has been increased, and there's a new emphasis on kebab-casing collection names. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 8
Configuration used: CodeRabbit UI
Files selected for processing (11)
- packages/admin/src/modules/AdminCampaign/AdminCampaignTable/ButtonUpdateModal/index.tsx (3 hunks)
- packages/admin/src/modules/AdminCampaign/ButtonCreateModal/CampaignForm/index.tsx (3 hunks)
- packages/user/src/lib/cep78/index.ts (3 hunks)
- packages/user/src/lib/cep78/types.ts (1 hunks)
- packages/user/src/lib/cep78/utils.ts (3 hunks)
- packages/user/src/modules/@core/nft-minter/hooks/use-create-nft.ts (3 hunks)
- packages/user/src/modules/@core/nft-minter/index.tsx (6 hunks)
- packages/user/src/modules/campaign/campaign-detail/index.tsx (4 hunks)
- packages/user/src/modules/campaign/feature-campaigns/components/CampaignCard/index.tsx (1 hunks)
- packages/user/src/modules/campaign/running-campaigns/index.tsx (1 hunks)
- packages/user/src/services/app/nft/types.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/user/src/modules/campaign/feature-campaigns/components/CampaignCard/index.tsx
Additional comments: 24
packages/admin/src/modules/AdminCampaign/AdminCampaignTable/ButtonUpdateModal/index.tsx (2)
15-15: The import statement has been correctly updated to remove
CheckboxElement
andMarkdownField
, which is consistent with the PR's objective to disable whitelist mode.110-128: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [94-128]
The remaining form elements and logic appear to be correctly retained and functional, aligning with the PR's objectives.
packages/admin/src/modules/AdminCampaign/ButtonCreateModal/CampaignForm/index.tsx (2)
62-65: Commented out
MarkdownField
rendering as part of disabling whitelist mode.89-91: Commented out
CheckboxElement
related to whitelist functionality.packages/user/src/lib/cep78/index.ts (3)
25-31: The addition of the
MintCallWasm
import and its usage in themintWithRegisterOwner
method aligns with the PR's objective to refactor the whitelist functionality and enhance the NFT minting process.213-227: The new
mintWithRegisterOwner
method in theCEP78Client
class correctly encapsulates the functionality to mint with the registration of the owner, using themint
method with theMintCallWasm
and additional arguments.272-277: The
mint
method has been correctly updated to handle an optionalcollectionName
parameter, which is consistent with the PR's objective to includecollectionName
in the minting process.packages/user/src/lib/cep78/types.ts (1)
- 116-119: The addition of the optional
collectionName
property to theMintArgs
interface is consistent with the PR's objective to update NFT minting processes and does not introduce any immediate issues. Ensure that all parts of the codebase that constructMintArgs
objects are aware of this new optional property and handle it accordingly.packages/user/src/lib/cep78/utils.ts (4)
10-14: The addition of
collectionName
toSignDeployNftParams
is consistent with the PR's objective to update NFT minting processes. Ensure that all functions usingSignDeployNftParams
are updated to handle the newcollectionName
parameter.25-29: The update to
signDeployNft
to includecollectionName
is consistent with the PR's objective. Verify that the logic withinsignDeployNft
correctly utilizescollectionName
and that all calls to this function pass the new parameter where required.41-52: The replacement of
mint
withmintWithRegisterOwner
insignDeployNft
aligns with the PR's objective to enhance NFT minting. Ensure thatmintWithRegisterOwner
is implemented correctly and that its usage here is appropriate.8-14: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [1-1]
Verify that the
use client
directive is appropriate for the context in whichutils.ts
is used, as it indicates client-side usage which may not be suitable for all environments.packages/user/src/modules/@core/nft-minter/hooks/use-create-nft.ts (3)
7-7: The addition of
_kebabCase
from "lodash-es/kebabCase" is consistent with the PR's objective to refactor the handling ofcollectionName
.59-59: The increase in
paymentAmount
aligns with the changes described in the PR.74-74: The transformation of
collectionName
using_kebabCase
before passing it tosignDeployNft
is consistent with the PR's objective to update the NFT minting process.packages/user/src/modules/@core/nft-minter/index.tsx (6)
20-23: The removal of
isAllowWhitelistUser
fromNFTMinterProps
and the component's destructuring aligns with the PR's objective to disable whitelist functionality.62-66: The addition of the
collectionName
parameter to themutate
function call is consistent with the PR's objective to update NFT minting processes.96-110: Commenting out the conditional block related to whitelist checking is consistent with the PR's objective to remove whitelist functionality.
118-118: The change in the HTML class attribute seems to be a minor cosmetic update, likely for consistency in styling.
75-78: The conditional rendering based on
isLoadingBalance
andisLoadingCompletedTransactions
enhances the UI's responsiveness to loading states.119-121: The new message for users who have reached the maximum entry limit seems to be an enhancement to user feedback.
packages/user/src/modules/campaign/campaign-detail/index.tsx (1)
- 162-169: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [103-166]
The commented-out JSX block and the hardcoded value '99' have been replaced with
nftCollection?.totalTokenSupply
. This aligns with the PR objective to remove whitelist functionality and to use dynamic values for total token supply.packages/user/src/modules/campaign/running-campaigns/index.tsx (1)
- 59-63: The conditional rendering of the "Running Campaigns" header based on the presence of data is a logical update that aligns with the PR's objective to refactor the UI components to be more dynamic.
packages/user/src/services/app/nft/types.ts (1)
- 16-20: The addition of the
collectionName
property toCreateTempNftParams
is consistent with the PR's objectives and the AI-generated summary. Ensure that all parts of the codebase that constructCreateTempNftParams
objects are updated to handle the new optional property correctly.
{/* <Box mt="1rem"> | ||
<Box mb="0.5rem">Description</Box> | ||
<MarkdownField /> | ||
</Box> | ||
</Box> */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code related to MarkdownField
. Consider removing it entirely unless there's a specific reason to keep it for future reference.
{/* <Box mt="1rem"> | ||
<CheckboxElement | ||
name="isOpenWhitelist" | ||
label="Is Open Whitelist" | ||
/> | ||
</Box> | ||
</Box> */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code related to isOpenWhitelist
. Consider removing it entirely unless there's a specific reason to keep it for future reference.
{/* <Box mt="1rem"> | ||
<CheckboxElement | ||
name="isAllowWhitelistUser" | ||
label="Is Allow Whitelist User To Mint" | ||
/> | ||
</Box> | ||
</Box> */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code related to isAllowWhitelistUser
. Consider removing it entirely unless there's a specific reason to keep it for future reference.
</div> | ||
{totalNFTs >= 99 ? ( | ||
</div> */} | ||
{totalNFTs >= (nftCollection?.totalTokenSupply || 999999) ? ( | ||
<div className="h-[72px]">SOLD OUT. Thanks for your support.</div> | ||
) : ( | ||
<> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isAllowWhitelistUser
prop in the NFTMinter
component may no longer be necessary given the PR's objective to remove whitelist functionality. Consider removing this prop if it's not used anymore.
|
||
import { Button } from "@mlem-user/components/ui/button"; | ||
import { SpinLoader } from "@mlem-user/components/ui/spin-loader"; | ||
import { CampaignStatusEnum } from "@mlem-user/enums/campaign-status"; | ||
import { formatDate } from "@mlem-user/lib/date"; | ||
import { cn } from "@mlem-user/lib/utils"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The font
variable is declared but not used in the JSX. If it's no longer needed, consider removing it to clean up the code.
import { LoadingButton } from '@mui/lab'; | ||
import Box from '@mui/material/Box'; | ||
import { | ||
CheckboxElement, | ||
DatePickerElement, | ||
FormContainer, | ||
SelectElement, | ||
} from 'react-hook-form-mui'; | ||
import { useQueryClient } from 'react-query'; | ||
|
||
import { StyledTextFieldElement } from './styled'; | ||
import MarkdownField from '../../AdminCampaignTable/ButtonUpdateModal/MarkdownField'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If MarkdownField
is no longer used in this file, consider removing the import to keep the code clean and avoid unnecessary dependencies.
@@ -8,6 +8,7 @@ import { Config } from "@mlem-user/config"; | |||
import { CEP78ClientInstance } from "."; | |||
|
|||
export type SignDeployNftParams = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The paymentAmount
parameter in SignDeployNftParams
is typed as an optional string, but it appears to be used as a number in the mintWithRegisterOwner
call. Verify that the type of paymentAmount
is consistent across all usages and consider updating the type to reflect how it is used.
import { CEP78ClientInstance } from "."; | ||
|
||
export type SignDeployNftParams = { | ||
collectionName: string; | ||
publicKeyHex: string; | ||
name: string; | ||
nftId: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [53-53]
The signDeployNftWithFee
function does not include the new collectionName
parameter. If this function is intended to be part of the NFT minting process updates, it should be modified to include and utilize collectionName
.
Summary by CodeRabbit
New Features
collectionName
during NFT creation and minting processes.Bug Fixes
Refactor
Style
Documentation
collectionName
property.Chores