[3Box Integration] Add 'Upload to 3Box' button to Avatar Builder

[think-in-universe]

Description

Based on the feature requested for 3box/3box#839, implement the GitCoin-3Box integration feature for Avatar Builder per the request of Kevin (@owocki).

We will take advantage of what we have achieved in GitCoin and 3Box integration in #5640, and extend the backup functionalities to Avatar Builder.

The design doc for this feature can be found here: https://hackmd.io/@L9ZnGph8Tbaio2GVX_z-EA/SylymWCAr

Refers/Fixes

#5660

Testing

Preview for this feature can be found below:

1. "upload to 3box" button

2. Verify the uploaded custom avatar data in 3Box Hub

(looks a bit messy due to the 3Box Hub UI)

[codecov]

Codecov Report

Merging #5667 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5667   +/-   ##
=======================================
  Coverage   28.87%   28.87%           
=======================================
  Files         270      270           
  Lines       23759    23759           
  Branches     3453     3453           
=======================================
  Hits         6861     6861           
  Misses      16629    16629           
  Partials      269      269

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbdaa14...e30ef77. Read the comment docs.

[sirlupinwatson]

Nice Project @think-in-universe i will give a new face to @owocki haha

[think-in-universe]

@Sabihashaik Haha. Awesome!!

I believe avatars are easier to be shared and collaboratively edited if it can be made open with data infrastructure like 3Box.

[owocki]

@think-in-universe awesome, im pumped for this!

kicking it over to @gitcoinco/engineers for review.. i believe that john from ethdenver wants this live before ETHDenver on 2/13

[thelostone-mc]

left a few minor comments about removing log otherwise LGTM

[think-in-universe]

@thelostone-mc thanks. the logs are removed.

[octavioamu]

we are merging #5947 that have a lot of duplicate code.
So this need a rebase/merge and will need also the migrations to be recreated.
Can you take a look @think-in-universe ?

[think-in-universe]

@octavioamu sure. I could help with merge and update migrations after we fix the issue with #5955 .

I understand we'd like to have the feature ready before Feb 13th, so I'll help with the code clean up and more testing once #5955 is resolved.

[think-in-universe]

@octavioamu merged with fix/3box-integration branch, and tested for saving profile and avatars to 3box locally, both works well.

[think-in-universe]

please note that 3box.min.js is now included via <script src="https://unpkg.com/3box/dist/3box.min.js"></script> in onboard.html and profile.html

please let me know if we prefer add 3box.min.js into app/assets/v2/js/lib/

[think-in-universe]

resolved the merge conflict

[think-in-universe]

could anyone take a look at the test failure of DashboardUtilsTest.test_get_bounty in travis? looks like a network error?

[sirlupinwatson]

could anyone take a look at the test failure of DashboardUtilsTest.test_get_bounty in travis? looks like a network error?

requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://rinkeby.infura.io/

Also incompatible version error.

[think-in-universe]

@Sirlupinwatson1 thanks~ is that an environment issue or related to my code change?

[sirlupinwatson]

@Sirlupinwatson1 thanks~ is that an environment issue or related to my code change?

This is what i can read :

   1 failed
     - app/dashboard/tests/test_dashboard_utils.py:62 DashboardUtilsTest.test_get_bounty

The command "pytest -p no:ethereum" exited with 1.

So this is a environment issue to me ? I can't guess on a answer. @danlipert did advertise me to add concrete comments only. I don't want to confuse you on your work. I just wanted to help.

but it say Error 403 :

self = <Response [403]>

�[1m def raise_for_status(self):�[0m
�[1m """Raises stored :class:HTTPError, if one occurred."""�[0m
�[1m �[0m
�[1m http_error_msg = ''�[0m
�[1m if isinstance(self.reason, bytes):�[0m
�[1m # We attempt to decode utf-8 first because some servers�[0m
�[1m # choose to localize their reason strings. If the string�[0m
�[1m # isn't utf-8, we fall back to iso-8859-1 for all other�[0m
�[1m # encodings. (See PR #3538)�[0m
�[1m try:�[0m
�[1m reason = self.reason.decode('utf-8')�[0m
�[1m except UnicodeDecodeError:�[0m
�[1m reason = self.reason.decode('iso-8859-1')�[0m
�[1m else:�[0m
�[1m reason = self.reason�[0m
�[1m �[0m
�[1m if 400 <= self.status_code < 500:�[0m
�[1m http_error_msg = u'%s Client Error: %s for url: %s' % (self.status_code, reason, self.url)�[0m
�[1m �[0m
�[1m elif 500 <= self.status_code < 600:�[0m
�[1m http_error_msg = u'%s Server Error: %s for url: %s' % (self.status_code, reason, self.url)�[0m
�[1m �[0m
�[1m if http_error_msg:�[0m
�[1m> raise HTTPError(http_error_msg, response=self)�[0m
�[1m�[31mE requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://rinkeby.infura.io/�[0m

�[1m�[31m../../../virtualenv/python3.7.1/lib/python3.7/site-packages/requests/models.py�[0m:940: HTTPError

[think-in-universe]

@Sirlupinwatson1 thanks. 😄

I rerun make pytest locally and all tests (including DashboardUtilsTest.test_get_bounty) passed. I think this might be a random failure.

And anyway the python test should not related to the change in the commit bbdaa14, which only made one-line change in html for resolving merge conflict.

[sirlupinwatson]

@Sirlupinwatson1 thanks. 😄

I rerun make pytest locally and all tests (including DashboardUtilsTest.test_get_bounty) passed. I think this might be a random failure.

And anyway the python test should not related to the change in the commit bbdaa14, which only made one-line change in html for resolving merge conflict.

I have this question in my mind, not sure if i can ask it here. But in the log files, there is a message saying about Interference for the TimeZone -8 , these warnings should be ignored ? I know also this is just a integration test but to be a perfect code we should not ignore those warnings ?

I would be happy if someone could answer to this question about the timezone log error, but also to give the right answer about the purpose of the main error it was randomly giving like @think-in-universe is saying.

Have a good day @think-in-universe !

[think-in-universe]

@Sirlupinwatson1 thanks~

@owocki hey Kevin, if what you want to get ready for ETHDenver is to backup the avatar builder data first, I think we could merge this PR first which will only save the custom_avatar fields into 3box in the avatar builder page, rather than save much more fields in the profile backup feature in the profile page.

[owocki]

thanks.. i dont have the cycles to dive into the weeds on this tho :-/

[octavioamu]

@owocki @think-in-universe I will test it locally

[octavioamu]

I don't see it on my 3box account but I believe is because the image is on localhost that is right?

[octavioamu]

Never mind I found it. Working perfectly

[octavioamu]

@Sirlupinwatson1 thanks. 😄
I rerun make pytest locally and all tests (including DashboardUtilsTest.test_get_bounty) passed. I think this might be a random failure.
And anyway the python test should not related to the change in the commit bbdaa14, which only made one-line change in html for resolving merge conflict.

I have this question in my mind, not sure if i can ask it here. But in the log files, there is a message saying about Interference for the TimeZone -8 , these warnings should be ignored ? I know also this is just a integration test but to be a perfect code we should not ignore those warnings ?

I would be happy if someone could answer to this question about the timezone log error, but also to give the right answer about the purpose of the main error it was randomly giving like @think-in-universe is saying.

Have a good day @think-in-universe !

that error is not from this pr actually @Sirlupinwatson1 is failing on travis in stable and master so we should check that separately from this review.

[think-in-universe]

@octavioamu cool.

is it necessary to merge this PR first when we're fixing #5955 with #5981?

[octavioamu]

@octavioamu cool.

is it necessary to merge this PR first when we're fixing #5955 with #5981?

not sure you tell me!! But the idea is having the 2 uploads for ethdenver

[sirlupinwatson]

@Sirlupinwatson1 thanks. 😄
I rerun make pytest locally and all tests (including DashboardUtilsTest.test_get_bounty) passed. I think this might be a random failure.
And anyway the python test should not related to the change in the commit bbdaa14, which only made one-line change in html for resolving merge conflict.

I have this question in my mind, not sure if i can ask it here. But in the log files, there is a message saying about Interference for the TimeZone -8 , these warnings should be ignored ? I know also this is just a integration test but to be a perfect code we should not ignore those warnings ?
I would be happy if someone could answer to this question about the timezone log error, but also to give the right answer about the purpose of the main error it was randomly giving like @think-in-universe is saying.
Have a good day @think-in-universe !

that error is not from this pr actually @Sirlupinwatson1 is failing on travis in stable and master so we should check that separately from this review.

Thanks for the answer @octavioamu , much appreciate.

From what i understand we need to update out things up and open-up another issue, but since you are all working on this issue and there is a rush , you will not open up a issue and just do it ?

I know this is running out of what is relevant here but i really do need to learn this and know why since i am learning to code. Thanks again.

[think-in-universe]

@octavioamu sure. I think we're mainly blocked for testing with the profile data in production.

as I mentioned in #5981 , it would be nice if we could run more testing in alpha, in order to collect the call stack (if met 500 error when export data), and query timeframe for large size profile.

[think-in-universe]

@octavioamu should we also merge this feature after #5981 is well tested?