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

Integration Branch for ENS, Duniter, Facebook, PersonhoodPassport on Trust Bonus Page #8569

Merged
merged 98 commits into from
Apr 20, 2021

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Mar 12, 2021

Description

Integration Branch for ENS, Duniter, Facebook on Trust Bonus Page under the "coming soon" area

Refers/Fixes

#7928
#7844
#7970
#8478

Testing
  • Tested Facebook which is working
  • Can't get Duniter to work + left comment on that PR
  • ENS working

developerfred and others added 30 commits November 7, 2020 18:58
 - add more validations
 - *same gitcoin user and duniter user uid
 - *social link "gitcoin.co" checker
 - *certificates check
 - *user is member check
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

left some comments and few questions

app/app/urls.py Outdated
dashboard.views.request_verify_facebook,
name='request_verify_facebook'
),
url(r'^api/v0.1/profile/verify_user_google', dashboard.views.verify_user_google, name='verify_user_google'),
Copy link
Contributor

Choose a reason for hiding this comment

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

this on already exist 2 lines down

app/app/urls.py Outdated Show resolved Hide resolved
showValidation: {
type: Boolean,
required: false,
'default': false
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I believe this should be default without quotes

this.verifyDuniter();
},
getUserHandle() {
this.githubHandle = trustHandle;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see where trustHandle is defined, is this code working? I suppose is a global variable defined out of vue context, if that the case, ok but if not then this will be undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

showValidation: {
type: Boolean,
required: false,
'default': false
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe is without quotes I never saw it with quotes, but probably doesn't affect the code.

Comment on lines +1060 to +1128
jQuery.fn.shake = function(interval, distance, times) {
interval = typeof interval == 'undefined' ? 100 : interval;
distance = typeof distance == 'undefined' ? 10 : distance;
times = typeof times == 'undefined' ? 3 : times;
var jTarget = $(this);

jTarget.css('position', 'relative');
for (var iter = 0; iter < (times + 1); iter++) {
jTarget.animate({ top: ((iter % 2 == 0 ? distance : distance * -1))}, interval);
}
return jTarget.animate({ top: 0}, interval);
};

$(document).on('click', '#gen_passport', function(e) {
e.preventDefault();
if (document.web3network != 'rinkeby') {
_alert('Please connect your web3 wallet to rinkeby + unlock it', 'error', 1000);
return;
}
const accounts = web3.eth.getAccounts();

$.when(accounts).then((result) => {
const ethAddress = result[0];
let params = {
'network': document.web3network,
'coinbase': ethAddress
};

$.get('/passport/', params, function(response) {
let status = response['status'];

if (status == 'error') {
_alert(response['msg'], 'error', 5000);
return;
}

let contract_address = response.contract_address;
let contract_abi = response.contract_abi;
let nonce = response.nonce;
let hash = response.hash;
let tokenURI = response.tokenURI;
var passport = new web3.eth.Contract(contract_abi, contract_address);

var callback = function(err, txid) {
if (err) {
_alert(err, 'error', 5000);
return;
}
let url = 'https://rinkeby.etherscan.io/tx/' + txid;
var html = `
<strong>Woo hoo!</strong> - Your passport is being generated. View the transaction <a href='` + url + `' target=_blank>here</a>.
<br><br>
<strong>Whats next?</strong>
<br>
1. Please add the token contract address (` + contract_address + `) as a token to your wallet provider.
<br>
2. Use the Trust you've built up on Gitcoin across the dWeb! Learn more at <a target=_blank href=https://proofofpersonhood.com>proofofpersonhood.com</a>

`;

$('.modal-body .subbody').html(html);
$('.modal-dialog').shake();
};

passport.methods.createPassport(tokenURI, hash, nonce).send({from: ethAddress}, callback);
});

});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

why not vue here? It's ok for now I think but we are creating tech debt for the near future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i wrestled with vue for a bit + just couldnt get it working. couldnt get the variables to scope correctly. happy to pair on rewriting it.

$(document).on('click', '#gen_passport', function(e) {
e.preventDefault();
if (document.web3network != 'rinkeby') {
_alert('Please connect your web3 wallet to rinkeby + unlock it', 'error', 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

should be rinkeby?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is behind an is_staff flag + not in mainnet yet

app/passport/views.py Show resolved Hide resolved
Copy link
Contributor

@gdixon gdixon left a comment

Choose a reason for hiding this comment

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

I've left a couple of comments for @developerfred to get the tests to pass...

One thing to note for when we deploy this: we will need to make sure that we have libsodium installed everywhere that this code will run.

app/dashboard/templates/profiles/trust-vue.html Outdated Show resolved Hide resolved
app/dashboard/tests/test_dashboard_views.py Show resolved Hide resolved
delimiters: [ '[[', ']]' ],
data: function() {
return {
showValidation: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

We're providing showValidation as a prop and defining a default value there - we don't need this ln 👍

@thelostone-mc thelostone-mc added this to Engineering-Review in PR Review Board Mar 22, 2021
@gdixon gdixon moved this from Engineering-Review to Waiting on contributor in PR Review Board Mar 22, 2021
@thelostone-mc
Copy link
Member

@owocki if you can get the comments addressed -> we can get it out this week else we'll have to push this back

@owocki
Copy link
Contributor Author

owocki commented Mar 23, 2021

@developerfred can you pls resolve the duniter comments? i can resolve the ones related to PoPP

@thelostone-mc
Copy link
Member

@owocki + @developerfred Could we get these fixed up by next week ?
This PR is starting to get stale and conflicts are starting to creep in

@owocki
Copy link
Contributor Author

owocki commented Apr 5, 2021

@developerfred if you cant get duniter fixed ( #7844 (comment) ) in the next day or two , i'm going to cut that code out + push the rest of this to deploy.

@developerfred
Copy link
Contributor

@developerfred if you cant get duniter fixed ( #7844 (comment) ) in the next day or two , i'm going to cut that code out + push the rest of this to deploy.

I ended up not being able to work on that, I had external impediments. But I am focused on solving this feature.

@developerfred
Copy link
Contributor

@owocki You can send facebook and ens, because it's ready. Because the Duniter is holding these deploys. there I am working on the feature attached.

@thelostone-mc
Copy link
Member

@developerfred could i get an ETA on when you'd be able to get this ready ?
If you aren't able to wrap it up by Monday -> I'd have to figure out how to undo your changes

Ideally, I'd want to avoid doing that cause you'd have to rework your changes

Copy link
Contributor

@gdixon gdixon left a comment

Choose a reason for hiding this comment

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

All LGTM!! 🚀

@thelostone-mc thelostone-mc merged commit cb19557 into master Apr 20, 2021
PR Review Board automation moved this from Waiting on contributor to Done Apr 20, 2021
@Elhamne
Copy link
Contributor

Elhamne commented Apr 24, 2021

Hi @thelostone-mc , I just see the Facebook integration has been merged; but I get Invalid App ID error on it.

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

Successfully merging this pull request may close these issues.

None yet

8 participants