-
Notifications
You must be signed in to change notification settings - Fork 43
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
Unlock Membership Tiers for Kickback #220
base: dev
Are you sure you want to change the base?
Conversation
Hi, @edsonayllon . Thank you for the submission. When I tested locally, Clicking the "Unlock membership? didn't do anything. I can also see "An error occurred on client" message (highlighted in red). I can also see some error messages on console when I first load the /create page cc @julien51 |
@makoto Yes, currently this PR isn't ready for review (marked as Work in Progress). I ran into this issue yesterday. I talked to @julien51 and it seems Unlock's paywall app is only deployed on Rinkeby testnet, however, the addresses provided were for Kovan. Julien's provided me keys for Rinkeby which I'll test soon. The Readme for the app includes instructions for Kovan, but |
@makoto This PR should be ready for review. 682d830 Network for Unlock is selected based on Dynamically selecting the network for Unlock was a bit tricky. Unlock is configured in To work around these, seeing the testnet version of Unlock would only be accessed during development or staging, and the mainnet verion of Unlock would be accessed in deployment, I set dynamically choosing the network based on the Environment Variable Again, currently Unlock Paywall is only compatible with Rinkeby and Mainnet, so testing should be done on Rinkeby. I've left the unlock keys for Mainnet empty, so make sure to enter those when your team creates them. |
public/index.html
Outdated
// unlock config must use var | ||
var unlockProtocolConfig = { | ||
locks, | ||
icon: 'https://unlock-protocol.com/static/images/svg/unlock-word-mark.svg', |
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.
This could be changed to a Kickback logo ;)
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.
Good idea. Changed in a60f5f7. It looks like only public urls are compatible with Unlock, so I used https://kickback.events/card.png.
Hi, Can you write an instruction on how to test on my local machine? I just did |
@makoto The red message box indicating wrong network is configured by |
Note: I changed the component I can change it back to a class component, if your team prefers. |
So is this the correct config?
I am still getting "Wrong network" on unlock. Also Is it possible to disable Unlock if |
@makoto Can you verify that you pulled the latest commits? Also, what are the steps you are taking to run the app? And what browser and OS? As for disabling Unlock, I can disable the paywall, and display the Create form by default if |
595316b Paywall is disabled if Is something wrong with https://kovan.api.kickback.events/graphql? It's throwing errors in the console. |
Hi, @edsonayllon kovan was down but now up. You can also test with https://rinkeby.kickback.events/create . Still not fully up and running but at least you can test the account creation page. |
umm, I deployed your branch to both rinkeby and kovan but both are showing spinning message.
Can you think of what would cause this? |
Actually I am seeing the same spinning on localhost if ENV=rinkeby (ENV=local will just render the event creation page as requested. Thanks!) |
Yup. I can think of a simple workaround to address that. Will do so after work. |
Ah ok. Would you mind adding the description of how to set things up in our README so that I can just follow that? |
…d instructions in README
Hi. For the deployed environment, I again deployed to https://rinkeby.kickback.events/create but getting the spinning image when connecting to either rinkeby or mainnet.
Can you check https://rinkeby.kickback.events/create and see if you are seeing the same spinning image? |
Something like this? Added a1166a7.
I tested and got the same thing building locally. The environment variable was written to pass with Fixed a1166a7. I added temporary keys for Mainnet, please replace these with your keys in |
|
||
useEffect(() => { | ||
// remove unlock in component unmount1 | ||
return () => { |
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.
Can this not be bundled with the other useEffect's return statement?
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.
You're right. dbc56b9
public/index.html
Outdated
Kovan lock is included for when kovan is supported by Unlock paywall | ||
NOTE: Replace Mainnet keys with keys Kickback owns | ||
*/ | ||
const locks = { |
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.
Is there any way to abstract these into the main src
directory. It's not very visible having the locks in the index.html.
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.
I don't think so. I looked through Unlock's documentation, and looked at the React example in Unlock's Github. They had this config in public/index.html
. I also don't think there's a way to import a file from src
to public
, else I'd be using env.json
instead of passing environment variables through package.json
. Unless @julien51 has any suggestions.
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.
Actually... I think this just adds a variable to window
. Let me test something
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.
Ok. Locks moved to config/env.json
in src
. Readme updated as well with instructions. 393de51
Hi,I tried both local and https://rinkeby.kickback.events/ with the latest commit (b76de81) and still stack at spinning despite placing "LOCKS" in the
|
That locks config is wrong. Please follow the README. |
src/routes/CreateEvent.js
Outdated
let unlock = window.unlockProtocol.blockchainData() | ||
let locks = Object.keys(unlock.locks).map(i => unlock.locks[i]) | ||
let bronze = locks.find(o => o.name.includes('Kickback Bronze')) | ||
let gold = locks.find(o => o.name.includes('Kickback Gold')) |
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.
Is this part used to determine which lock I became the member of?
Does unlock.locks
returns all the locks only purchased by the specific Ethereum account? I tested with 4 different metamask accounts where account 1~3 bought gold and account 4 bought bronze though it kept showing gold no matter which ETH account I switch into.
When I put console.log({locks})
under account 4 it had both bronze and gold hence looks like it picked gold regardless.
I wonder if unlock.locks
just returns all the available locks, rather than the lock purchased by the purchaser (=event organiser). cc @julien51
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.
So, it is undocumented, but the way to get the locks for which the user has a key is to do this:
const data = window.unlockProtocol.blockchainData()
if (data.keys) {
// They should be indexed by lock.
// Let's assume there is only one valid.
const lockAddress = Object.keys(data.keys)[0]
}
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.
@makoto I wonder if unlock.locks just returns all the available locks, rather than the lock purchased by the purchaser (=event organiser).
You're right.
@julien51 So, it is undocumented, but the way to get the locks for which the user has a key is to do this: [...]
data.keys
returned all available locks as well for me. Tested by purchasing on a new account.
I used data.transaction
instead to get the latest purchase. Then displayed the membership by the lock purchased. I left the full algorithm commented in updateUnlockUser()
.
Should display the correct membership owned now.
LGTM @edsonayllon @julien51 . Thank you so much for being super patient with my constant feedback! |
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.
All good for me too!
@makoto Pleaase send me the lock addresses that you'd like to enable credit card for!
Let's merge this so we can release the bounty! ;) |
Issue number
#209
Description
See issue #209 linked.
In this PR, unlock displays page, as seen in the image below, requesting membership to view requested content. Once verified, form to deploy kickback contract is displayed to the user with membership status.
List of features added/changed
/create
How Has This Been Tested?
Testing done in Rinkeby. If account does not have membership token, form is not displayed. Purchase of membership token tested. Form displays after purchase. Switching to an account with no membership token shows restricted page again, as expected.
React-Router has also been updated to include
history
as a hook. Browsing other pages show routing is still working as expected.Screenshots (if appropriate):
Checklist: