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

🤳🏻 Enable collectibles and NFT as profile picture #12615

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

shivekkhurana
Copy link
Contributor

@shivekkhurana shivekkhurana commented Sep 22, 2021

fixes #12613

Summary

  • Enable NFTs view (based on selected network, only works with mainnet and rinkyby)

  • Allow setting NFTs as Profile picture

  • View NFT details

  • TODO in next PR: Make NFT txns

Platforms

  • Android
  • iOS

Areas that maybe impacted

  • Profile pictures
Functional
  • 1-1 chats
  • public chats
  • group chats
  • wallet / transactions
  • new account
  • user profile updates

status: Ready

@status-github-bot status-github-bot bot added this to REVIEW in Pipeline for QA Sep 22, 2021
@status-im-auto
Copy link
Member

status-im-auto commented Sep 22, 2021

Jenkins Builds

Click to see older builds (52)
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ 1b1edc0 #1 2021-09-22 10:06:48 ~16 min android 📦apk 📲
✖️ 1b1edc0 #1 2021-09-22 10:10:44 ~20 min ios 📦ipa 📲
✖️ 1b1edc0 #1 2021-09-22 10:11:37 ~21 min android-e2e 📦apk 📲
6c7788f #2 2021-09-27 11:51:05 ~18 sec android 📄log
6c7788f #2 2021-09-27 11:51:05 ~19 sec ios 📄log
6c7788f #2 2021-09-27 11:51:13 ~27 sec android-e2e 📄log
✔️ 6c9468e #3 2021-09-27 12:11:17 ~17 min ios 📦ipa 📲
✔️ 6c9468e #3 2021-09-27 12:12:22 ~18 min android-e2e 📦apk 📲
✔️ 6c9468e #3 2021-09-27 12:12:40 ~18 min android 📦apk 📲
✔️ f4cbbdc #4 2021-09-27 12:18:54 ~14 min ios 📦ipa 📲
✔️ f4cbbdc #4 2021-09-27 12:22:58 ~18 min android-e2e 📦apk 📲
✔️ f4cbbdc #4 2021-09-27 12:22:59 ~18 min android 📦apk 📲
✔️ 4c60daa #5 2021-09-27 15:30:42 ~11 min android 📦apk 📲
✔️ 4c60daa #5 2021-09-27 15:31:05 ~12 min android-e2e 📦apk 📲
✔️ 4c60daa #5 2021-09-27 15:32:16 ~13 min ios 📦ipa 📲
4d94e8c #6 2021-09-29 11:39:19 ~6 min ios 📄log
✔️ 4d94e8c #6 2021-09-29 11:46:26 ~13 min android 📦apk 📲
✔️ 4d94e8c #6 2021-09-29 11:49:19 ~16 min android-e2e 📦apk 📲
4d94e8c #7 2021-09-29 12:05:26 ~6 min ios 📄log
646ca28 #8 2021-10-01 10:20:16 ~5 min ios 📄log
✔️ 646ca28 #7 2021-10-01 10:27:51 ~12 min android 📦apk 📲
✔️ 646ca28 #7 2021-10-01 10:32:20 ~17 min android-e2e 📦apk 📲
ba3b9d4 #9 2021-10-01 10:38:53 ~4 min ios 📄log
✔️ ba3b9d4 #8 2021-10-01 10:47:09 ~13 min android-e2e 📦apk 📲
✔️ ba3b9d4 #8 2021-10-01 10:48:52 ~15 min android 📦apk 📲
e05af9c #10 2021-10-01 11:08:21 ~10 min ios 📄log
✔️ e05af9c #9 2021-10-01 11:10:27 ~13 min android 📦apk 📲
✔️ e05af9c #9 2021-10-01 11:10:31 ~13 min android-e2e 📦apk 📲
✔️ 2d3c7af #11 2021-10-01 12:13:34 ~20 min ios 📦ipa 📲
✔️ 8b1cd24 #11 2021-10-02 14:29:41 ~13 min android-e2e 📦apk 📲
✔️ 8b1cd24 #11 2021-10-02 16:21:31 ~13 min android 📦apk 📲
✔️ 8b1cd24 #12 2021-10-03 12:49:07 ~16 min ios 📦ipa 📲
✔️ 8476227 #12 2021-10-06 07:45:01 ~12 min android 📦apk 📲
✔️ 8476227 #12 2021-10-06 07:46:44 ~14 min android-e2e 📦apk 📲
✔️ 8476227 #13 2021-10-06 07:49:24 ~16 min ios 📦ipa 📲
✖️ d8d1206 #13 2021-10-06 08:43:10 ~12 min android-e2e 📦apk 📲
✖️ d8d1206 #13 2021-10-06 08:44:05 ~13 min android 📦apk 📲
✖️ d8d1206 #14 2021-10-06 08:46:21 ~15 min ios 📦ipa 📲
✖️ b34623a #14 2021-10-06 09:19:02 ~12 min android-e2e 📦apk 📲
✖️ b34623a #14 2021-10-06 09:20:31 ~14 min android 📦apk 📲
✖️ b34623a #15 2021-10-06 09:31:04 ~24 min ios 📦ipa 📲
✖️ 4347d3b #15 2021-10-06 09:35:51 ~15 min android-e2e 📦apk 📲
✖️ 4347d3b #15 2021-10-06 09:36:59 ~16 min android 📦apk 📲
✖️ 4347d3b #16 2021-10-06 09:37:11 ~16 min ios 📦ipa 📲
✖️ bf7fef0 #16 2021-10-08 09:39:07 ~14 min android-e2e 📦apk 📲
✖️ bf7fef0 #16 2021-10-08 09:39:11 ~14 min android 📦apk 📲
✖️ e5209b1 #17 2021-10-11 14:50:12 ~12 min android 📦apk 📲
✖️ e5209b1 #17 2021-10-11 14:51:38 ~14 min android-e2e 📦apk 📲
✖️ e5209b1 #18 2021-10-11 14:56:29 ~18 min ios 📦ipa 📲
✔️ fef77b7 #18 2021-10-12 10:38:34 ~14 min android 📦apk 📲
✔️ fef77b7 #19 2021-10-12 10:40:13 ~16 min ios 📦ipa 📲
✔️ fef77b7 #18 2021-10-12 10:41:20 ~17 min android-e2e 📦apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ c407fe9 #19 2021-10-13 09:07:02 ~13 min android 📦apk 📲
✔️ c407fe9 #19 2021-10-13 09:08:42 ~15 min android-e2e 📦apk 📲
✔️ c407fe9 #20 2021-10-13 09:15:18 ~21 min ios 📦ipa
✔️ d21538a #20 2021-10-13 09:20:10 ~14 min android-e2e 📦apk 📲
✔️ d21538a #20 2021-10-13 09:20:16 ~14 min android 📦apk 📲
✔️ d21538a #21 2021-10-13 09:21:05 ~15 min ios 📦ipa

@status-github-bot status-github-bot bot moved this from REVIEW to CONTRIBUTOR in Pipeline for QA Sep 23, 2021
@shivekkhurana shivekkhurana marked this pull request as ready for review September 27, 2021 11:54
@shivekkhurana
Copy link
Contributor Author

shivekkhurana commented Sep 27, 2021

📸 View Screenshot

Simulator Screen Shot - iPhone 11 - 2021-09-27 at 16 40 52

🔗 Figma Design

@hesterbruikman
Copy link
Contributor

@shivekkhurana Could you please make a minor edit to the Collectibles empty state:
It currently says "you will share your wallet and IP address", this should be "You will share your account and IP address". For consistency. Also, this is my bad, I proposed the copy using 'wallet' earlier.

@hesterbruikman
Copy link
Contributor

Also curious about the toast message. What flexibility do we have there when it comes to formatting? e.g. Are emoji's an option?

@shivekkhurana
Copy link
Contributor Author

Also curious about the toast message. What flexibility do we have there when it comes to formatting? e.g. Are emoji's an option?

Everything is allowed. All React components are valid toasts, however I cannot promise anything until I check. If you want emoji's send me a design (or a rough description) and I'll try it out.

@hesterbruikman
Copy link
Contributor

Also curious about the toast message. What flexibility do we have there when it comes to formatting? e.g. Are emoji's an option?

Everything is allowed. All React components are valid toasts, however I cannot promise anything until I check. If you want emoji's send me a design (or a rough description) and I'll try it out.

Awesome! I wasn't thinking of styling the component tbh, but thinking it's a good opportunity for more casual language. We don't use that often, because it's easy to go wrong. But this is a fun low-risk context:

"Profile picture updated😎"

@flexsurfer flexsurfer moved this from CONTRIBUTOR to E2E Tests in Pipeline for QA Sep 28, 2021
@status-im-auto
Copy link
Member

97% of end-end tests have passed

Total executed tests: 67
Failed tests: 2
Passed tests: 65
IDs of failed tests: 6645,6237 

Failed tests (2)

Click to expand
  • Rerun tests

  • 1. test_restore_account_migrate_multiaccount_to_keycard, id: 6645

    Device 1: *Tap on found Button*
    Device 1: *Find Button by accessibility id:* `back-button`

    Device 1: Button by accessibility id: `back-button` is not found on the screen

    Device sessions

    2. test_fetching_balance_after_offline, id: 6237

    Device 1: **Toggling airplane mode**
    Device 1: *Find AirplaneModeButton by accessibility id:* `Airplane mode`

    Device 1: AirplaneModeButton by accessibility id: `Airplane mode` is not found on the screen

    Device sessions

    Passed tests (65)

    Click to expand

    1. test_image_in_one_to_one_send_save_reply_timeline, id: 6305
    Device sessions

    2. test_add_account_to_multiaccount_instance_generate_new, id: 6224
    Device sessions

    3. test_can_add_existing_ens_on_mainnet, id: 5502
    Device sessions

    4. test_keycard_can_see_all_transactions_in_history, id: 6291
    Device sessions

    5. test_sign_message_and_2tx_in_batch_and_transactions_filters_from_daap, id: 5342
    Device sessions

    6. test_add_and_delete_watch_only_account_to_multiaccount_instance, id: 6244
    Device sessions

    7. test_open_blocked_secure_not_secure_inlalid_offline_urls, id: 6210
    Device sessions

    8. test_open_public_chat_using_deep_link, id: 5396
    Device sessions

    9. test_offline_add_new_group_chat_member, id: 3998
    Device sessions

    10. test_send_non_english_message_to_newly_added_contact_on_different_networks, id: 5315
    Device sessions

    11. test_can_see_balance_and_all_transactions_history_on_cellular, id: 5314
    Device sessions

    12. test_insufficient_funds_wallet_positive_balance, id: 5412
    Device sessions

    13. test_delete_close_all_tabs, id: 5390
    Device sessions

    14. test_create_new_group_chat_messaging_pn_delivered, id: 3994
    Device sessions

    15. test_send_token_with_7_decimals, id: 5350
    Device sessions

    16. test_delete_chats_via_delete_button_rejoin, id: 5387
    Device sessions

    17. test_send_transaction_set_recipient_options, id: 6328
    Device sessions

    18. test_keycard_sign_message_and_transactions_from_daap, id: 6251
    Device sessions

    19. test_recover_account_from_new_user_seedphrase, id: 6296
    Device sessions

    20. test_keycard_can_recover_keycard_account_card_pairing, id: 5758
    Device sessions

    21. test_block_user_from_public_chat, id: 5786
    Device sessions

    22. test_send_audio_message_with_push_notification_check, id: 6316
    Device sessions

    23. test_redirect_to_public_chat_tapping_tag_message_fetch_more_history, id: 5675
    Device sessions

    24. test_edit_delete_message_in_one_to_one_and_public_chats, id: 695843
    Device sessions

    25. test_ens_mentions_pn_and_nickname_in_public_and_1_1_chats, id: 6226
    Device sessions

    26. test_keycard_send_eth_from_wallet_to_address, id: 6289
    Device sessions

    27. test_keycard_create_login_restore_unlock_same_seed, id: 5689
    Device sessions

    28. test_add_account_to_wallet_private_key_and_seed_phrase, id: 6272
    Device sessions

    29. test_offline_is_shown_messaging_1_1_chat_sent_delivered, id: 5310
    Device sessions

    30. test_pair_devices_sync_one_to_one_contacts_nicknames_public_chat, id: 5762
    Device sessions

    31. test_long_press_to_delete_chat, id: 5319
    Device sessions

    32. test_copy_and_paste_messages, id: 5317
    Device sessions

    33. test_open_transaction_on_etherscan_copy_tx_hash, id: 5384
    Device sessions

    34. test_unread_messages_counter_public_chat, id: 5360
    Device sessions

    35. test_send_eth_from_wallet_to_address_incorrect_password, id: 5308
    Device sessions

    36. test_mobile_data_usage_complex_settings, id: 6228
    Device sessions

    37. test_start_chat_with_ens_mention_in_one_to_one, id: 5403
    Device sessions

    38. test_decline_transactions_in_1_1_chat_push_notification_changing_state, id: 6265
    Device sessions

    39. test_send_eth_in_1_1_chat_transaction_push, id: 6253
    Device sessions

    40. test_install_pack_and_send_sticker, id: 5782
    Device sessions

    41. test_switch_users_special_char_password_and_add_new_account_logcat, id: 5356
    Device sessions

    42. test_manage_assets, id: 5341
    Device sessions

    43. test_browser_managing_bookmarks, id: 6633
    Device sessions

    44. test_open_chat_by_pasting_chat_key_check_invalid_chat_key_cases, id: 5304
    Device sessions

    45. test_request_stt_from_daap, id: 5309
    Device sessions

    46. test_can_use_purchased_stickers_on_recovered_account, id: 5783
    Device sessions

    47. test_dapps_permissions, id: 5738
    Device sessions

    48. test_account_recovery_with_uppercase_whitespaces_seed_phrase_special_char_passw_logcat, id: 5394
    Device sessions

    49. test_send_eth_to_ens_in_chat, id: 6279
    Device sessions

    50. test_share_copy_contact_code_and_wallet_address, id: 5323
    Device sessions

    51. test_wallet_set_up, id: 5335
    Device sessions

    52. test_send_transaction_with_custom_token, id: 6208
    Device sessions

    53. test_home_view, id: 5379
    Device sessions

    54. test_logcat_backup_recovery_phrase, id: 5419
    Device sessions

    55. test_request_and_receive_stt_in_1_1_chat_offline, id: 6263
    Device sessions

    56. test_set_profile_picture, id: 6646
    Device sessions

    57. test_add_and_remove_mention_contact_with_nickname_from_public_chat, id: 5332
    Device sessions

    58. test_keycard_request_stt_from_daap, id: 6249
    Device sessions

    59. test_restore_account_from_mnemonic_to_keycard, id: 6240
    Device sessions

    60. test_pass_phrase_validation, id: 5363
    Device sessions

    61. test_keycard_send_eth_in_1_1_chat, id: 6293
    Device sessions

    62. test_send_and_open_links_with_previews, id: 5373
    Device sessions

    63. test_reactions_to_message_in_chats, id: 6315
    Device sessions

    64. test_push_notification_1_1_chat_no_pn_activity_center, id: 6283
    Device sessions

    65. test_public_chat_messaging_emojis_timestamps, id: 5313
    Device sessions

    @status-im-auto
    Copy link
    Member

    100% of end-end tests have passed

    Total executed tests: 2
    Failed tests: 0
    Passed tests: 2
    

    Passed tests (2)

    Click to expand

    1. test_restore_account_migrate_multiaccount_to_keycard, id: 6645
    Device sessions

    2. test_fetching_balance_after_offline, id: 6237
    Device sessions

    @churik
    Copy link
    Member

    churik commented Sep 29, 2021

    @shivekkhurana
    I can't see any images of collectibles on my Android 10.

    IMAGE 2021-09-29 14:23:04

    IOS build can't be built.
    Can you please take a look?

    @shivekkhurana
    Copy link
    Contributor Author

    shivekkhurana commented Sep 29, 2021

    @churik Do you have an error for ios builds?

    @churik
    Copy link
    Member

    churik commented Sep 29, 2021

    @shivekkhurana shivekkhurana force-pushed the feat/nft-enable-and-nft-as-pfp branch 2 times, most recently from e05af9c to 2d3c7af Compare October 1, 2021 11:52
    @jakubgs
    Copy link
    Member

    jakubgs commented Oct 1, 2021

    No idea why but doing make nix-update-pods in a separate PR worked and fixed issues in this one: #12676

    @churik churik moved this from E2E Tests to IN TESTING in Pipeline for QA Oct 1, 2021
    @churik
    Copy link
    Member

    churik commented Oct 1, 2021

    @shivekkhurana thank you for PR!

    ISSUE 1: Missing translation in settings

    IMAGE 2021-10-01 16:29:07

    ISSUE 2: Still can see amount of collectibles on Ropsten network, but they are not tappable

    Steps:

    1. restore multiaccount with collectible from seed phrase(PR build by default is using Ropsten)
    2. Enable Collectibles

    Expected result:
    Collectibles are disabled, it is explained somehow in UI (i.e. Collectibles are shown only on mainnet)

    Actual result:

    IMAGE 2021-10-01 16:41:35
    OS: IOS, Android

    ISSUE 3: Handling of collectibles without images

    • 3.1. Set some placeholder for collections without images

    Monosnap 2021-10-01 16-52-20

    • 3.2. Set placeholders for collectibles without placeholders

    Monosnap 2021-10-01 16-53-31

    • 3.3. Disable Use as profile photo for collectibles without images as it can't be set

    Monosnap 2021-10-01 16-56-08

    ISSUE 4: Number of collectibles on OpenSea website and in status app is different

    Steps:

    1. add watch-only account to multiaccount (0xcf2272205cc0cf96Cfbb9Dd740BD681D1E86901E)
    2. count collectibles
    3. tap Open in Opensea and compare value

    Expected result:
    amount of collectibles should be the same (12)
    Actual result:
    12 in OpenSea, 13 in Status
    The redundant is MegaCryptoPolis, it is not tappable from wallet.
    OS: IOS, Android

    ISSUE 5 (question): why the background color is black when you set collectible as profile photo?

    IMAGE 2021-10-01 17:18:46

    @shivekkhurana
    Copy link
    Contributor Author

    shivekkhurana commented Oct 6, 2021

    1 - Fixed ✅

    2 - Fixed ✅

    3 - Added placeholders Adding an svg renderer 🟡

    Realised that svgs are not as problematic as empty images or mp4s. Added conditions for them.
    Only one item left:
    [ ] 3.2. Set placeholders for collectibles without placeholders need design help to get placeholder images

    4 - Checking but looks bad - most likely related to APIs 😢

    5 - Okay to go with black bg for this release ✅

    So background is also visible in chat list and chat. I think it's fine to release as is, background doesn't look too bad and wouldn't appear frequently

    Screenshot 2021-10-06 at 1 26 33 PM

    @churik
    Copy link
    Member

    churik commented Oct 6, 2021

    @shivekkhurana thank you for the fixes!

    ISSUE4

    is reproducible; it is not only that particular account, but I also can see differences for another accounts (i.e. simona.eth on opensea has 179 items, in status.im 183 items are shown)

    ISSUE5 No sorting, so no way to find smth you need in random list

    Better to sort them alphabetically.

    I suppose we can fix separately ISSUE 5, but not ISSUE 4.

    src/status_im/multiaccounts/core.cljs Outdated Show resolved Hide resolved
    src/status_im/ui/screens/chat/styles/photos.cljs Outdated Show resolved Hide resolved
    src/status_im/ui/screens/wallet/collectibles/views.cljs Outdated Show resolved Hide resolved
    @status-im-auto
    Copy link
    Member

    100% of end-end tests have passed

    Total executed tests: 1
    Failed tests: 0
    Passed tests: 1
    

    Passed tests (1)

    Click to expand

    1. test_restore_account_migrate_multiaccount_to_keycard, id: 6645
    Device sessions

    @shivekkhurana
    Copy link
    Contributor Author

    ISSUE4

    Anthony and I did some detective work to find what the issue is.

    There are two aspects of the OpenSea api:
    1- A list of collections
    2- Actual assets list (can get all assets or filter by collection id)

    The failing collections (MCP in this case) have 0 assets returned although the collection says 1 asset available. So the app stays stuck at loading state (grayish box with no icon). Rainbow works fine because the don't query collections API and simply fetch all assets (ie. fat payload) and then compute collections from all assets.

    We face a problem because we fetch assets on demand, and it gets stuck when there are 0 assets in the collection.

    • One possible "easy" solution is to show an error when there are 0 assets in a collection DONE
    • Other possible solution is to fetch all assets upfront but this will hamper UX because we will be loading all assets upfront. Dropped - Because fat payloads are no good 🙅🏻

    ISSUE5 No sorting, so no way to find smth you need in random list

    Better to sort them alphabetically DONE


    Also addressed comments from Roman. I have not enabled it in release version yet, but I think its good to go in develop. That will allow a broader review.

    @shivekkhurana
    Copy link
    Contributor Author

    @qoqobolo Hey, this is ready to be re-re-tested.

    @qoqobolo
    Copy link
    Contributor

    @shivekkhurana thanks for the fixes and for all this great work!
    Checked the latest fixes and basic flows, everything looks good to me.
    PR can be merged.

    @qoqobolo qoqobolo moved this from IN TESTING to MERGE in Pipeline for QA Oct 12, 2021
    @shivekkhurana shivekkhurana force-pushed the feat/nft-enable-and-nft-as-pfp branch 2 times, most recently from c407fe9 to d21538a Compare October 13, 2021 09:05
    New UI and re-frame handlers for setting pfp from url
    Add fake chain-id, connect the pfp upload backend to UI
    Add toast on setting pfp, new-new-new UI
    Show assets based on selected network
    Add horzontal padding to traits card
    a11y ids and pr review changes
    Added emoji in toast, fix chain id nill issue
    Restore Podfile
    Fix fix podfile
    Add placeholder images for collections, hide option to set pfp when image url is empty or ends in mp4 or svg
    Improvise missing designs
    Fix paddings
    New nu placeholders
    Sort collections by name
    Kinda fix issue 4
    Fix lint
    
    Signed-off-by: Shivek Khurana <shivek@status.im>
    @shivekkhurana shivekkhurana merged commit 389ce66 into develop Oct 13, 2021
    Pipeline for QA automation moved this from MERGE to DONE Oct 13, 2021
    @shivekkhurana shivekkhurana deleted the feat/nft-enable-and-nft-as-pfp branch October 13, 2021 09:06
    @shivekkhurana shivekkhurana mentioned this pull request Oct 14, 2021
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Archived in project
    Development

    Successfully merging this pull request may close these issues.

    Enable NFTs on Mobile
    8 participants