Skip to content

Substrate startkit GUI milestone #1 delivery#30

Merged
Noc2 merged 3 commits intow3f:masterfrom
ivorrr:master
Sep 2, 2020
Merged

Substrate startkit GUI milestone #1 delivery#30
Noc2 merged 3 commits intow3f:masterfrom
ivorrr:master

Conversation

@ivorrr
Copy link
Copy Markdown
Contributor

@ivorrr ivorrr commented Aug 10, 2020

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • The invoice form 📝 has been filled out for this milestone.
  • This pull request is done by the same account, which is responsible for the pull request of the accepted application.
  • BTC address changed in agreement with the web3 Foundation team. In the case of acceptance, the payment will be transferred to the initial BTC payment address.

@Noc2
Copy link
Copy Markdown
Contributor

Noc2 commented Aug 10, 2020

Thanks for the delivery. We will look into it as soon as possible.

@mmagician
Copy link
Copy Markdown
Contributor

Hi @ivorrr

In general the delivery looks good, though I do have a few questions/comments:

Licence:

  • Missing Apache license from the repo

Wireframes:

  • Where do the pre-built packages (Wallet, Currencies etc) come from? Is MVP Workshop currently maintaining that repository? Or are these just placeholders?
  • What happens when a user requests a package? Does the request go to MVP Workshop who might/might not decide to implement it?
  • What is the connection between deploying a package to GitHub and selecting Your package -> submit package? Is the user prompted to enter a GitHub url or something along these lines?

Backend & GUI application

  • Please include instructions in the README on how to run the app that you're already building, even if it's just the bare-bones.
  • Would you mind having a walkthrough with me of the repo structure & tooling choices. What I have in mind is a short videa call with your team. Please reach out to me directly on the Element chat: @marcin:web3.foundation to coordinate, if possible.

Documentation

  • It seems somewhat counter-productive to maintain a separate version of description for each pallet. I assume the Google Document that you provided contains the information directly from https://crates.parity.io/, right? As this documentation would at some point end up on your GitHub, I presume, then I would avoid this sort of duplication. It's just an idea, we can discuss possible solution if you want.
  • I realise that the original https://marketplace-staging.substrate.dev/ is perhaps not the most user friendly reference. As a suggestion to the previous point, perhaps it would be worth to setup a collaboration with Parity & improve on this, so that the whole ecosystem benefits as a whole, but specifically your project, which could then take the information directly from only one source.

Github Authentication

  • It would be beneficial to include a small tutorial on how to progress with this. Perhaps it would be obvious once I get a sample app running

@ivorrr
Copy link
Copy Markdown
Contributor Author

ivorrr commented Aug 13, 2020

Thanks, @mmagician for the feedback, really appreciated!

  1. Missing Apache license from the repo
    We've just added Apache-2.0, let us know if you think something else will be more suitable.

  2. Where do the pre-built packages (Wallet, Currencies etc) come from? Is MVP Workshop currently maintaining that repository? Or are these just placeholders?
    Our idea was to help users with having some "pre-built" packages (templates) meaning we would check for dependencies and basically combine a few pallets that work together for a specific use case (e.g. wallet) into a template.
    For now, we did tech research and will start combining pallets into templates in the second part of Aug (when we plan to start with 2nd phase of the project), but in a nutshell - we will be checking automatically for the pallet compatibility and if they can work under the package together before populating it in the list. If some template became outdated or incompatible, we will automatically remove it from the list of templates, but initial push will be done by us manually to check compatibility and group different pallets in the templates.

  3. What happens when a user requests a package? Does the request go to MVP Workshop who might/might not decide to implement it?
    We can have Parity's e-mail there for submitting a request and CC substrate@mvpworkshop.co mail group for new requests so we can assist in maintaining pre-built packages over the course of time. We can also trigger some event in the background and send requests to the community as well if you see there's something existing we could integrate with.

  4. What is the connection between deploying a package to GitHub and selecting Your package -> submit package? Is the user prompted to enter a GitHub url or something along these lines?
    This is one of the iterations we had until we finished with the UI Design later on, so please feel free to check "UI Design" page in Figma, there should be the final and latest version of the design prototype we have tested with a few blockchain developers to validate that it is intuitive enough. You will see some style, copy and layout changes there as well in comparison with the initial wireframes. We are open to improve copy and adjust if needed but wanted to show you the design process and iterations we had along the way.

  5. Please include instructions in the README on how to run the app that you're already building, even if it's just the bare-bones.
    Done, tnx.

  6. Would you mind having a walkthrough with me of the repo structure & tooling choices.
    Sure, no problem. We suggest having a call during the second milestone when we move forward with the development since the 1st milestone was related to the UX and UI design mostly and together with the research and user testing afterward, this was our main focus for the 1st delivery.

  7. It seems somewhat counter-productive to maintain a separate version of description for each pallet.
    Agree, the doc was created for our team to be able to grasp all the different information regarding the pallets, but the final documentation will be available on the GitHub as part of the second milestone and also we can reference and link it to the existing pallet descriptions.

  8. I realise that the original https://marketplace-staging.substrate.dev/ is perhaps not the most user friendly reference. As a suggestion to the previous point, perhaps it would be worth to setup a collaboration with Parity & improve on this
    We're 100% with this one :) We can also combine it with the app we are building right now since we are using the latest Substrate brand guidelines and have information architecture and layout in place to support the marketplace as well.
    We could basically have one more tab in the sidebar to list all the pallets available and sort them by types and categories, but let us know how you want to proceed with this one and should we submit another project for that idea. In short, we like it and would totally be up to improve marketplace as well.

  9. It would be beneficial to include a small tutorial on how to progress with this. Perhaps it would be obvious once I get a sample app running.
    Once we will be approaching the end of the second milestone, we had in plan to basically create video tutorial covering complete user flow and all the options we will have in the app. We plan to do it in the way that main components described could be cut into smaller videos for the purpose of describing single components such as GitHub auth.

Thanks once again Marcin for the great comments, we really did enjoy working on the 1st milestone and looking forward to the 2nd and digging deeper into the development.

Let us know if there's anything else needed from our side before proceeding further.

@mmagician
Copy link
Copy Markdown
Contributor

@ivorrr Thanks a lot for the clarifications.

I've found the instructions under the develop branch, I assume that's the one I should be using? I am a little confused regarding:
3. Setup the postgres database
What I've done was createdb substrate_startkit and then filled in DB_NAME=substrate_startkit in the .env file. I assume the rest of the settings are default. Does setting up the postgres db involve something else that I should execute?
5. [HOST_ADDRESS] => Since I'm running this locally, would this just be localhost? I assume I also need to place HOST_ADDRESS=localhost in my .env file?
8. yarn seed - yarn does not seem to have a seed command - perhaps I am missing some dependency?

Thanks for support!

@ivorrr
Copy link
Copy Markdown
Contributor Author

ivorrr commented Aug 14, 2020

Hi @mmagician

Yes, the Develop branch is up to date. It would be best if you used it.

To answer your questions:
3.
DB_NAME=substrate_startkit (you choose this one)
DB_PASSWORD=db user password
DB_PORT=db port on your machine, default is 5432
DB_USER=The user who is the owner of the db (Should be the one that ran 'createdb' command, if you're not sure try using "postgres")

*Also you should set Github creds in .env you can find the guidance in the readme file

HOST_ADDRESS=localhost -is fine

Sorry, my mistake I gave you a wrong command, instead of "yarn seed" try running "sequelize db:seed:all"

@mmagician
Copy link
Copy Markdown
Contributor

@ivorrr Thanks for the answers.

I am able to see some tables created in the DB as well as directories for migrations and seeders. However, I'm getting {"message":"Not Found"} when I try to access localhost:3001.

Is there something else that I'm missing?

@pajicf
Copy link
Copy Markdown

pajicf commented Aug 19, 2020

Hi @mmagician!

It seems like you've successfully started the backend.

You're getting {"message":"Not Found"}, I'm guessing, because you're probably trying to access
the GET localhost:3001/ route which doesn't actually exist.

If you're on develop branch, you can find the available routes inside
/src/api/v1/index.ts file with the HTTP methods, alongside with their definitions
inside /definitions directory.

The routes would be:
GET /status
GET /me
GET /auth/github
GET /pallets
GET /pallets/:palletName

Note that the /pallets routes aren't actually part of the first milestone and are
still work in progress.

Please let us know if there's anything else needed from our side,
seems like we are slowly getting to talk about the phase 2 deliverables
so we would like to proceed with the development.

@pajicf
Copy link
Copy Markdown

pajicf commented Aug 25, 2020

Hi @mmagician,

If there is any more information that we should provide, please let us know. We would like to start digging deeper into development and proceed with Milestone 2. Thank you!

@mmagician
Copy link
Copy Markdown
Contributor

@pajicf Thanks for the update from your side. We're currently quite busy with the evaluations, but I will try to get back in touch regarding your milestone 1 by the end of this week.

@mmagician
Copy link
Copy Markdown
Contributor

@pajicf So am I right in assuming that the following should give me Server is alive message, as per https://github.com/MVPWorkshop/substrate-startkit-gui-api/blob/develop/src/api/v1/routes/status.route.ts?

curl -XGET http://localhost:3001/status

I'm still getting a Not Found here.

@pajicf
Copy link
Copy Markdown

pajicf commented Aug 25, 2020

@mmagician
Yes, the assumption is correct, just the address of the route you're trying to fetch isn't.

All API routes are pre-indexed with /api/v1
So in your example, instead of http://localhost:3001/status you should try fetching http://localhost:3001/api/v1/status

@mmagician
Copy link
Copy Markdown
Contributor

@pajicf That worked, thanks, I'm successfully fetching the JSONs.

Just one last thing, regarding the github authentication: I'm seeing this kind of error in the application logs when accessing:
http://localhost:3001/api/v1/auth/github/ in the browser:

AuthorizationError: The redirect_uri MUST match the registered callback URL for this application.

I filled in "Authorization callback URL" for the GitHub OAuth application details with: http://localhost:3001/api/v1/auth/github/callback

Also I seem to have the right credentials in my .env file. Am I missing something else?

Regardless of this, I suggest that your team proceed to the next milestone. I'm positive that we can figure this one out and once we do, it'll just be a formality to accept the delivery, given that we have reviewed all the designs earlier.

@pajicf
Copy link
Copy Markdown

pajicf commented Aug 25, 2020

@mmagician That's great!

The .env value (HOST_ADDRESS) and the one in the Github dashboard probably don't match, which should be the cause of the error.

Can you try changing the value of HOST_ADDRESS in .env to include http:// and the port number 3001,
it should be looking something like this: http://localhost:3001.

We will start working on the second milestone, but please let us know when we can officially close 1st milestone and proceed with the payment (note that BTC address is changed from the initial application)

@mmagician
Copy link
Copy Markdown
Contributor

@pajicf Great, that worked, thanks!

I will start working on the official approval now.
I've also made a small PR to update the README & .env.example files as per our conversation here.

@mmagician
Copy link
Copy Markdown
Contributor

@pajicf @ivorrr
There are some minor admin adjustments that need to be done in order to close off milestone #1:

  • the deliverables have changed from the initial application, namely:
    • GitHub authentication is part of this delivery
    • Testing & integration have been moved to milestone 2
    • Set React app project structure & start working on UI components -> The UI components have only been developed in Figma, as far as I could see.
  • payment address has changed

Therefore, before we can process this, we need to have a reevaluation & approval by the committee, as per Open Grants Program document. You'll need to open a PR against this file.

May I also suggest a change to how you split the payment, namely to be 50-50 between the milestones. While it's great that you've integrated GitHub auth already, the workload involved there is considerably less than CI & testing (for which more app logic would be needed, anyway, to be useful). I doubt that such a "swap" be deemed acceptable by the committee under the same payout conditions.

Hope this helps, let me know should you have any further questions.

ivorrr added a commit to ivorrr/Open-Grants-Program that referenced this pull request Aug 26, 2020
Based on the [conversation ](w3f/Grant-Milestone-Delivery#30 (comment)) we had with @mmagician on the [milestone 1 deliverables ](w3f/Grant-Milestone-Delivery#30), we are opening a new PR against this grant. Here's a [link for the initial PR](w3f#32).

Changes included in this PR:
- Deliverables
  - GitHub authentication is part of the first milestone
  - Testing & integration have been moved to milestone 2
  - Working on UI components was moved to milestone 2
- Payment
  - The payment address has been changed
  - The first milestone instead of 1.8 BTC is now 1.5 BTC
  - The second milestone instead of 1.2 BTC is now 1.5 BTC

We also submitted new invoice through the following [form](https://docs.google.com/forms/d/e/1FAIpQLSeZUWVZjrSGUKO38E4_gf2Amd_0QT-HAW0-8wXmI7m54Bggrw/viewform), based on the above changes.
@pajicf
Copy link
Copy Markdown

pajicf commented Aug 26, 2020

Hi @mmagician,

Thank you for the answer, we agree with everything you wrote above. Hence we opened new PR against this file and adjusted milestones 1 and 2 deliverables. We have also changed the payment address (as previously mentioned) and payment amounts between milestones to be 50-50.

Thank you once again!

@mmagician
Copy link
Copy Markdown
Contributor

@pajicf I saw your PR, thanks. Once it's approved, I will finish the milestone review.

@mmagician
Copy link
Copy Markdown
Contributor

@ivorrr @pajicf Thanks for the work done for Milestone 1. I have submitted a positive evaluation (please have a look at the comments there) that has now been merged: #34.

I'll be looking forward to the next milestone!

@Noc2 Noc2 merged commit e8b3d82 into w3f:master Sep 2, 2020
@RouvenP
Copy link
Copy Markdown

RouvenP commented Sep 11, 2020

hi @ivorrr @pajicf we sent a test transaction. Could you confirm if received?

@ivorrr
Copy link
Copy Markdown
Contributor Author

ivorrr commented Sep 11, 2020

Hi @RouvenP we received a transaction of 0.01btc from 3GF.................4i2Jy

@RouvenP
Copy link
Copy Markdown

RouvenP commented Sep 11, 2020

@ivorrr thanks for confirming. Payment is sent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants