Skip to content

Conversation

@zlayine
Copy link
Contributor

@zlayine zlayine commented May 1, 2025

PR Type

Enhancement


Description

  • Add signingAccount field to advanced forms

  • Include signingAccount in API payloads

  • Update GraphQL mutations to accept signingAccount

  • Initialize signingAccount ref in components


Changes walkthrough 📝

Relevant files
Enhancement
13 files
BatchMint.vue
Include `signingAccount` input in batch mint form               
+9/-0     
CreateCollection.vue
Add `signingAccount` to collection creation form                 
+8/-0     
DeleteBeamSlideover.vue
Add `signingAccount` input to beam delete slide-over         
+10/-0   
ApproveCollectionSlideover.vue
Include `signingAccount` in collection approval form         
+9/-0     
FreezeSlideover.vue
Add advanced `signingAccount` field to freeze UI                 
+9/-0     
AccountsFuelTankSlideover.vue
Add `signingAccount` option to fuel tank accounts               
+9/-0     
CancelListingSlideover.vue
Include `signingAccount` in cancel listing slide-over       
+9/-0     
ApproveTokenSlideover.vue
Add advanced `signingAccount` to token approval form         
+9/-0     
index.ts
Pass `signingAccount` in API service methods                         
+4/-0     
beam.ts
Extend Beam API with `signingAccount` variable                     
+4/-0     
collection.ts
Add `signingAccount` support in Collection API                     
+8/-0     
fueltank.ts
Include `signingAccount` in FuelTank API calls                     
+11/-0   
Freeze.ts
Update `Freeze` mutation to accept `signingAccount`           
+2/-1     
Additional files
43 files
marketplace.ts +5/-0     
token.ts +8/-0     
BatchSetAttribute.vue +9/-0     
BatchTransfer.vue +2/-1     
CreateBeam.vue +9/-0     
CreateFuelTank.vue +1/-0     
CreateListing.vue +10/-0   
CreateToken.vue +10/-0   
ExpireBeamSlideover.vue +10/-0   
UpdateBeamSlideover.vue +10/-0   
AttributesCollectionSlideover.vue +8/-0     
DestroyCollectionSlideover.vue +8/-0     
MutateCollectionSlideover.vue +9/-0     
UnapproveCollectionSlideover.vue +9/-0     
ThawSlideover.vue +9/-0     
TransferBalanceSlideover.vue +1/-0     
ConsumptionFuelTankSlideover.vue +9/-0     
DestroyFuelTankSlideover.vue +9/-0     
DispatchFuelTankSlideover.vue +9/-0     
FreezeFuelTankSlideover.vue +9/-0     
MutateFuelTankSlideover.vue +1/-0     
RuleSetsFuelTankSlideover.vue +11/-0   
FillListingSlideover.vue +9/-0     
FinalizeAuctionSlideover.vue +9/-0     
PlaceBidSlideover.vue +9/-0     
AttributesTokenSlideover.vue +9/-0     
BurnTokenSlideover.vue +1/-0     
InfuseTokenSlideover.vue +1/-0     
MintTokenSlideover.vue +9/-1     
MutateTokenSlideover.vue +9/-0     
TransferTokenSlideover.vue +1/-0     
UnapproveTokenSlideover.vue +9/-0     
RemoveAllAttributes.ts +2/-1     
Thaw.ts +2/-1     
CancelListing.ts +2/-2     
PlaceBid.ts +2/-2     
ApproveToken.ts +2/-1     
CreateToken.ts +2/-1     
MintToken.ts +2/-1     
MutateToken.ts +2/-1     
RemoveTokenAttribute.ts +2/-1     
SetTokenAttribute.ts +2/-1     
UnapproveToken.ts +2/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @zlayine zlayine self-assigned this May 1, 2025
    @github-actions
    Copy link

    github-actions bot commented May 1, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Import

    The new v-if="useAppStore().advanced" directives require useAppStore to be imported in the component. Verify that useAppStore is available to avoid runtime errors.

    <FormInput
        v-if="useAppStore().advanced"
        v-model="signingAccount"
        name="signingAccount"
        label="Signing Account"
        description="The wallet used to sign and broadcast the transaction. By default, this is the wallet daemon."
    />
    GraphQL Syntax

    The field name in the mutation is prefixed with $ (i.e., $signingAccount) which is invalid syntax. It should be signingAccount: $signingAccount.

    export default `mutation PlaceBid($listingId: String!, $price: BigInt!, $signingAccount: String, $idempotencyKey: String) {
        PlaceBid(listingId: $listingId, price: $price, $signingAccount: $signingAccount, idempotencyKey: $idempotencyKey) {
          id

    @github-actions
    Copy link

    github-actions bot commented May 1, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix PlaceBid argument name

    The GraphQL field name must match the server schema. Replace the incorrect
    $signingAccount: argument with signingAccount: to avoid syntax errors.

    resources/js/graphql/mutation/marketplace/PlaceBid.ts [1-3]

     export default `mutation PlaceBid($listingId: String!, $price: BigInt!, $signingAccount: String, $idempotencyKey: String) {
    -  PlaceBid(listingId: $listingId, price: $price, $signingAccount: $signingAccount, idempotencyKey: $idempotencyKey) {
    +  PlaceBid(listingId: $listingId, price: $price, signingAccount: $signingAccount, idempotencyKey: $idempotencyKey) {
         id
       }
     }`;
    Suggestion importance[1-10]: 9

    __

    Why: The GraphQL argument must use signingAccount: instead of $signingAccount: to match the server schema and avoid syntax errors.

    High
    General
    Use store instance in template

    Avoid calling the store function in the template. Instead, import useAppStore in
    <script setup>, assign it to a constant, and reference its advanced property for
    better performance and consistency.

    resources/js/components/batch/BatchMint.vue [41-47]

     <FormInput
    -    v-if="useAppStore().advanced"
    +    v-if="appStore.advanced"
         v-model="signingAccount"
         name="signingAccount"
         label="Signing Account"
         description="The wallet used to sign and broadcast the transaction. By default, this is the wallet daemon."
     />
     
    +<script setup>
    +import { useAppStore } from '~/store';
    +const appStore = useAppStore();
    +// ...
    +</script>
    +
    Suggestion importance[1-10]: 6

    __

    Why: Using useAppStore() directly in the template can cause unnecessary function calls; importing the store in <script setup> and referencing appStore.advanced improves performance.

    Low
    Conditionally include signingAccount

    Guard against sending an empty or default signingAccount. Only include it in the
    payload when the user has explicitly set a non‐empty value.

    resources/js/components/batch/BatchMint.vue [198-204]

    -const res = await ApiService.batchMint(
    -  formatData({
    -    collectionId: collectionId.value,
    -    idempotencyKey: idempotencyKey.value,
    -    skipValidation: skipValidation.value,
    -    recipients: mints.value.map((item) => item.values),
    -    signingAccount: signingAccount.value,
    -  })
    -);
    +const payload = {
    +  collectionId: collectionId.value,
    +  idempotencyKey: idempotencyKey.value,
    +  skipValidation: skipValidation.value,
    +  recipients: mints.value.map((item) => item.values),
    +  ...(signingAccount.value && { signingAccount: signingAccount.value }),
    +};
    +const res = await ApiService.batchMint(formatData(payload));
    Suggestion importance[1-10]: 5

    __

    Why: Guarding the payload to exclude empty signingAccount avoids sending unnecessary default values and keeps the API call cleaner.

    Low

    @zlayine zlayine force-pushed the bugfix/PLA-2208/add-signing-account branch from de6be4c to 6c10e05 Compare May 5, 2025 16:37
    @zlayine zlayine merged commit e719875 into master May 5, 2025
    4 checks passed
    @zlayine zlayine deleted the bugfix/PLA-2208/add-signing-account branch May 5, 2025 16:54
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Development

    Successfully merging this pull request may close these issues.

    2 participants