[Explorer] Add Metadata page #168

matextrem · 2022-07-20T20:51:36Z

Summary

Closes #160

Added a new Metadata page where you can create a new app data document and also upload it to IPFS

To Test

Open the page metadata

You can complete the form and then hit GENERATE METADATA DOC button
After the app data generation, you can also upload it to IPF by clicking UPLOAD APP DATA TO IPFS button

…upload it to IPFS

coveralls · 2022-07-20T21:23:07Z

Pull Request Test Coverage Report for Build 2883630218

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 43.824%

Totals
Change from base Build 2882905680:	0.0%
Covered Lines:	2158
Relevant Lines:	4174

💛 - Coveralls

socket-security · 2022-07-20T21:27:05Z

Socket Security Report

👍 No new dependency issues detected in pull request

Socket.dev scan summary

Issue	Status
Did you mean?	✅ no new possible package typos
Install scripts	✅ no new install scripts
Telemetry	✅ no new telemetry
Troll package	✅ no new troll packages
Malware	✅ no new malware
Native code	✅ no new native modules

Powered by socket.dev

matextrem · 2022-07-20T22:22:50Z

@ramirotw could you try build the lock.file and push it ? I'm trying to do it locally but it's failing on my side.

src/components/orders/DetailsTable/index.tsx

src/apps/explorer/pages/Metadata/config.ts

tsconfig.json

github-actions · 2022-07-21T00:17:04Z

🔭 Explorer: Explorer test app
🔒 Safe Swap App: Trade test app (add to your safe apps)
📈 Trade: Trade test app
📚 Storybook: Component stories

elena-zh · 2022-07-21T12:44:23Z

Hey, this is my early review of the PR.
I know, that UI has not been implemented yet, but anyways I will leave here some my proposals. Maybe they might be useful:

I think it would be nice to add hints as tooltips near a field's label
As for environments hint, we could add 'pr' to the list, as it is also used in Cowswap app for appdata
I think it would be great to show errors in red. Besides, I think it is better to validate fields on FE side first. WDYT?
Nice to adjust the page to look better in a mobile view

And here is the list of issues that can be fixed before the UI part:

Based on the issue description, a user can check the Quote or Refferrer section: Then, for the metadata, user can pick any of the choices of referrer and quote. Maybe a checkbox to enable them? Currently, both sections are enabled.
Quote version field name is incorrect (shows 'quote' instead of the 'version'. The field should be mandatory and show the latest version (read-only)
Address field should be marked as mandatory field when Referrer section is selected
There is no validation on the address field when I use upper or lower-case letters. I'm not sure, however, if it is needed. But I have to notice that this validation exists in Explorer when search for a user details
Again, I'm not sure if it is an issue, but the fields have a different order in 'referrer' section in a metadata info
Slippage Bips field should be marked as a mandatory one (when the section is selected)
I think we should make a user aware of errors when there is something wrong with uploading data to IPFS (show an error banner somehow).
The same with a success notification (show 'successfully uploaded' message/banner or so)

Thanks

ramirotw · 2022-07-22T01:01:16Z

It would be helpful to add a description field on the form using the description prop from the schema:

src/apps/explorer/pages/Metadata/config.ts

src/apps/explorer/pages/Metadata/styled.ts

elena-zh · 2022-08-08T13:50:31Z

Hey @matextrem , @alongoni changes LGTM!

Still, I see that the issue with periods in the end of sentences is not fixed (will be addressed in cowprotocol/app-data#10 a bit later).

And there is no path to the page, but will be implemented soon.

Also I have found #175 issue that @ramirotw can take a look into a bit later.

So I'm approving the current PR.

alfetopito

Overall I'm happy with the behaviour and looks good for merging.

Doesn't mean there isn't more stuff to address :)
I'll write it down in more details tomorrow, when I'll also finish the code review.

alfetopito · 2022-08-08T16:53:48Z

src/apps/explorer/ExplorerApp.tsx

@@ -132,6 +140,7 @@ const AppContent = (): JSX.Element => {
          <Route path={pathPrefix + '/address/:address'} exact component={UserDetails} />
          <Route path={pathPrefix + '/tx/:txHash'} exact component={TransactionDetails} />
          <Route path={pathPrefix + '/search/:searchString?'} exact component={SearchNotFound} />
+          <Route path={pathPrefix + '/metadata'} exact component={AppDataDetails} />


Suggested change

<Route path={pathPrefix + '/metadata'} exact component={AppDataDetails} />

<Route path={pathPrefix + '/appdata'} exact component={AppDataDetails} />

alfetopito

Couple of nitpicks regarding the code but nothing major.
All of them can be addressed in follow up PRs.

I'm happy merging as is and iterate.

As I mentioned today, this page will be hidden until the new menu is ready, so it's fine to be merged even if some things still need to be adjusted.

I also mentioned to the team some more information to be displayed in each of the new types to give more context to users.

src/apps/explorer/pages/AppData/FormPage.tsx

src/apps/explorer/pages/AppData/config.tsx

src/apps/explorer/pages/AppData/index.tsx

matextrem · 2022-08-17T13:39:07Z

@alfetopito fixed!

Cc: @elena-zh

elena-zh · 2022-08-18T10:00:27Z

Hey @matextrem , could toy please describe what exactly I need to retest except changing the page path to /appdata and changing the tab name to 'Encode'?

alfetopito · 2022-08-18T10:32:30Z

src/const.ts

-    acc[networkId] = new CowSdk(networkId, {
-      isDevEnvironment: true,
-    })
+    acc[networkId] = new CowSdk(networkId)


Any reason for changing this?

I haven't checked it throughly, but I suspect it might be the cause of duplicated orders, like seen in this batch

I would avoid touching this if possible and leave that up to Ramiro's/mine PR which is adding Goerli support and also updating the SDK that does this

I think this broke when I merged from develop. Will fix this

matextrem · 2022-08-18T12:56:25Z

Hey @matextrem , could toy please describe what exactly I need to retest except changing the page path to /appdata and changing the tab name to 'Encode'?

@elena-zh I made some changes in the logic for the encode tab, so maybe we could test it again to see nothing is broken

elena-zh · 2022-08-18T14:52:30Z

Seems like works as expected.
Anyways, I will completely retest this page when it is going to be released.
Thanks!

Add Metadata page allows you to generate a new app data document and …

f61d8af

…upload it to IPFS

matextrem added the Enhancement New feature or request label Jul 20, 2022

matextrem added this to the 2.11.0 milestone Jul 20, 2022

matextrem requested review from alfetopito and ramirotw July 20, 2022 20:51

matextrem self-assigned this Jul 20, 2022

matextrem added 3 commits July 20, 2022 17:51

fix typo

d15fedd

Update package.json

86b31bc

Update yarn.lock

8d2ba3e

fix yarn.lock

d5b16ea

ramirotw reviewed Jul 21, 2022

View reviewed changes

src/components/orders/DetailsTable/index.tsx Outdated Show resolved Hide resolved

src/apps/explorer/pages/Metadata/config.ts Outdated Show resolved Hide resolved

tsconfig.json Outdated Show resolved Hide resolved

update lock file

57e2203

matextrem and others added 2 commits July 21, 2022 17:24

Improve validation + Get schema from appData module

df9e28e

update lock file

2bca150

ramirotw reviewed Jul 22, 2022

View reviewed changes

src/apps/explorer/pages/Metadata/config.ts Outdated Show resolved Hide resolved

alongoni reviewed Jul 22, 2022

View reviewed changes

src/apps/explorer/pages/Metadata/config.ts Outdated Show resolved Hide resolved

alongoni reviewed Jul 22, 2022

View reviewed changes

src/apps/explorer/pages/Metadata/styled.ts Outdated Show resolved Hide resolved

Improve metadata form + Use schema from appData module

587e289

matextrem requested review from alongoni and ramirotw July 22, 2022 20:23

Improve styles

36df94e

matextrem marked this pull request as ready for review July 22, 2022 21:30

matextrem added 2 commits July 22, 2022 18:47

update icon

2e37128

Update yarn.lock

903e1f6

alongoni and others added 6 commits August 5, 2022 12:43

fix notification message on mobile

842c1f9

revert decodeAppData layout

6306d19

fix decode layout v2

45e6482

wording AppData

1d1ef76

wording AppData v2

995ed15

Add appData hash validation

2ebf550

elena-zh mentioned this pull request Aug 8, 2022

Sometimes the app is crashed when open AppData page #175

Closed

elena-zh approved these changes Aug 8, 2022

View reviewed changes

alongoni added 2 commits August 8, 2022 11:23

add missing periods

2d5d622

add more periods

c7b9f65

alfetopito reviewed Aug 8, 2022

View reviewed changes

alfetopito approved these changes Aug 9, 2022

View reviewed changes

matextrem added 2 commits August 17, 2022 01:27

Refactor code

6de8d73

Merge branch 'develop' into 160/metadata-page

e95f63f

alfetopito reviewed Aug 18, 2022

View reviewed changes

matextrem added 3 commits August 18, 2022 11:53

Merge branch 'develop' into 160/metadata-page

3435a95

Update SDK instance

d188546

Fix type for appDataDoc

9d54484

matextrem merged commit 6c7e19c into develop Aug 18, 2022

matextrem deleted the 160/metadata-page branch August 18, 2022 18:03

ramirotw mentioned this pull request Aug 18, 2022

AppData / Affiliate Code #20

Closed

5 tasks

ramirotw removed this from the 2.12.0 milestone Aug 18, 2022

This was referenced Aug 19, 2022

AppData: Show more/less button triggers data search that runs in BE even when switch to another tab #190

Closed

Expand error message when decoding is failed #191

Closed

henrypalacios mentioned this pull request Aug 19, 2022

Release v2.12.0 #189

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Explorer] Add Metadata page #168

[Explorer] Add Metadata page #168

matextrem commented Jul 20, 2022

coveralls commented Jul 20, 2022 •

edited

Loading

socket-security bot commented Jul 20, 2022 •

edited

Loading

matextrem commented Jul 20, 2022

github-actions bot commented Jul 21, 2022

elena-zh commented Jul 21, 2022

ramirotw commented Jul 22, 2022

elena-zh commented Aug 8, 2022

alfetopito left a comment

alfetopito Aug 8, 2022

alfetopito left a comment

matextrem commented Aug 17, 2022

elena-zh commented Aug 18, 2022 •

edited

Loading

alfetopito Aug 18, 2022

matextrem Aug 18, 2022

matextrem commented Aug 18, 2022

elena-zh commented Aug 18, 2022

	<Route path={pathPrefix + '/metadata'} exact component={AppDataDetails} />
	<Route path={pathPrefix + '/appdata'} exact component={AppDataDetails} />

[Explorer] Add Metadata page #168

[Explorer] Add Metadata page #168

Conversation

matextrem commented Jul 20, 2022

Summary

To Test

coveralls commented Jul 20, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2883630218

💛 - Coveralls

socket-security bot commented Jul 20, 2022 • edited Loading

Socket Security Report

matextrem commented Jul 20, 2022

github-actions bot commented Jul 21, 2022

elena-zh commented Jul 21, 2022

ramirotw commented Jul 22, 2022

elena-zh commented Aug 8, 2022

alfetopito left a comment

Choose a reason for hiding this comment

alfetopito Aug 8, 2022

Choose a reason for hiding this comment

alfetopito left a comment

Choose a reason for hiding this comment

matextrem commented Aug 17, 2022

elena-zh commented Aug 18, 2022 • edited Loading

alfetopito Aug 18, 2022

Choose a reason for hiding this comment

matextrem Aug 18, 2022

Choose a reason for hiding this comment

matextrem commented Aug 18, 2022

elena-zh commented Aug 18, 2022

coveralls commented Jul 20, 2022 •

edited

Loading

socket-security bot commented Jul 20, 2022 •

edited

Loading

elena-zh commented Aug 18, 2022 •

edited

Loading