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

Release 004 #131

Merged
merged 6 commits into from
Jan 12, 2023
Merged

Release 004 #131

merged 6 commits into from
Jan 12, 2023

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Jan 4, 2023

TS error:

Unsafe member access .maker on an `any` value

need little help to edit MarketplaceOfferWithdrawnEvent event

PR type

  • Bugfix

What's new?

Before submitting Pull Request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

Screenshot

Capture d’écran 2023-01-04 à 9 20 02 AM

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

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

Otherwise oki

@@ -288,9 +288,11 @@ export async function handleOfferPlace(context: Context): Promise<void> {
// logger.debug(`offer: ${JSON.stringify({ ...event, price: String(event.amount), expiresAt: String(event.expiresAt) }, null, 2)}`)
const id = createTokenId(event.collectionId, event.sn);
const entity = ensure<NE>(await get(context.store, NE, id));
plsBe(real, entity);

// entity doesn't need to exist
Copy link
Member

Choose a reason for hiding this comment

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

NFT has to exist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

But It can fail on unknown foreign key in DB 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

But It can fail on unknown foreign key in DB 🤔

which foreign key? it only concerns Offers, if you find a particular case I would like to see it

I think we have no choice here you can check #127 to know why I've made this particular change

Copy link
Member

Choose a reason for hiding this comment

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

from Offers to NFTs
But if you try that and it does not fail I am happy to try that in rococo

if (event.isV55) {
const { who: caller, class: classId, instance: instanceId } = event.asV55;
return { collectionId: classId.toString(), sn: instanceId.toString(), caller: addressOf(caller) };
return { collectionId: classId.toString(), sn: instanceId.toString(), caller: addressOf(caller), maker: addressOf(ctx.event.call?.args.maker) };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

any idea how to fix this line? or have something a bit cleaner

Copy link
Member

Choose a reason for hiding this comment

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

I think this is sufficient

@vikiival
Copy link
Member

vikiival commented Jan 5, 2023

Also @roiLeo I think this update should go to main imo

@roiLeo
Copy link
Contributor Author

roiLeo commented Jan 5, 2023

Also @roiLeo I think this update should go to main imo

well depend how you want to handle release, I'm more into testing it on Rococo then release it on 005.

By the way, I wanted to ask you if it was possible to have a certain right on the 004 branch since it's testnet

@vikiival
Copy link
Member

vikiival commented Jan 5, 2023

By the way, I wanted to ask you if it was possible to have a certain right on the 004 branch since it's testnet

Sure!

@vikiival vikiival merged commit 01862cc into kodadot:release-004 Jan 12, 2023
@roiLeo
Copy link
Contributor Author

roiLeo commented Jan 12, 2023

LETS GO!
908

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants