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

Feat: Marketing contract #1280

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Feat: Marketing contract #1280

wants to merge 5 commits into from

Conversation

WaDadidou
Copy link
Collaborator

@WaDadidou WaDadidou commented Sep 18, 2024

This contract allows to

  • Update and Get the Banner (home)
  • Update and Get the Highlitghted News (home and top menu)
  • Update and Get the Live Collections (launchpad and top menu)
  • Update and Get the Upcoming Collections (launchpad)
  • Update and Get the Highlitghted Collections (launchpad)
  • Update and Get the Admin

It concerns these elements on the front

On the homepage https://app.teritori.com/

  • Banner (One for now):

image

  • News:

image

On the Launchpad https://app.teritori.com/launchpad

  • Highlighted collections:

image

  • Live collections:

image

  • Upcoming collections:

image

Misc

All the Update #[msg(exec)] can only be executed by the admin (Compares the sender address and the admin_addr)

Succesfully instantiated on teritori-testnet.
The starting config is set with the wallet used to instantiate, see https://github.com/TERITORI/teritori-dapp/pull/1280/files#diff-2159b5311d04a13bc07f3f5fabc962a474f43619cbf420476e9f78d235bc526eR7

Hm.. But the goal is to set a DAO as admin

Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for teritori-dapp ready!

Name Link
🔨 Latest commit adf1db5
🔍 Latest deploy log https://app.netlify.com/sites/teritori-dapp/deploys/66fee9fbcd5fe80008d1a13b
😎 Deploy Preview https://deploy-preview-1280--teritori-dapp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for testitori ready!

Name Link
🔨 Latest commit adf1db5
🔍 Latest deploy log https://app.netlify.com/sites/testitori/deploys/66fee9fb14984c0009b2b210
😎 Deploy Preview https://deploy-preview-1280--testitori.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@WaDadidou WaDadidou mentioned this pull request Sep 18, 2024
@WaDadidou WaDadidou marked this pull request as draft September 18, 2024 18:45
@WaDadidou WaDadidou self-assigned this Sep 18, 2024
@WaDadidou WaDadidou force-pushed the feat-marketing-contract branch 2 times, most recently from 3e66b54 to 0b654a8 Compare September 18, 2024 20:13
@WaDadidou WaDadidou marked this pull request as ready for review September 20, 2024 01:00
@@ -0,0 +1,4 @@
{
"admin_adrr": "tori1yzjgaql23yxlwxxvmszssg23w3f8k6k2q75jss"
Copy link
Collaborator

Choose a reason for hiding this comment

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

admin_addr could be better

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The value or the key ?

self.news.save(ctx.deps.storage, index as u64, &news_item)?;
}

Ok(Response::new().add_attributes(attributes))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we attributes is not use in earlier code then you can write it here to make the code more readable, we dont have to traceback to see the value of attributes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok I see

self.live_collections.save(ctx.deps.storage, index as u64, &live_collection)?;
}

Ok(Response::new().add_attributes(attributes))
Copy link
Collaborator

Choose a reason for hiding this comment

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

   Ok(Response::default()
        .add_attribute("action", "update_live_collections")
 )

make the code more readable. and seems that Response::default() add some needed default value to response also

}

#[msg(query)]
pub fn get_live_collections(&self, ctx: QueryCtx) -> StdResult<Vec<MarketingCollectionPreview>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we merge 3 queries: live, highlighted, upcoming => 1 query function and use a switch case to return the according data ? we can prevent the repeated code => maintain easier, contract size smaller, easy to add more query with the same kind of data

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After reflexion, handling "Which collections are shown in the Launchpad" doesn't make sense.
The collections shown are handled by the Marketplace, automatically, no need to choose.
I'll remove the collections stuff from the marketing contract

Copy link

Choose a reason for hiding this comment

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

It's very nice that you are working!

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

Successfully merging this pull request may close these issues.

3 participants