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

Create Tokenguard - Milestone 1 & 2.md #1160

Merged
merged 5 commits into from
Jun 10, 2024
Merged

Conversation

caaaml
Copy link
Contributor

@caaaml caaaml commented Mar 23, 2024

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, invoices must be submitted and payments will be transferred to the Polkadot AssetHub and/or fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#1944 < please fill this in with the PR number of your application.

@caaaml
Copy link
Contributor Author

caaaml commented Mar 23, 2024

Hey @keeganquigley !

Please find here the delivery for the Dashboard composer. As promised, here you can test the solution on real-life data:
https://demo-dc-app.tokenguard.io/

Happy to hear your thoughts! Also, we've modified our USDT wallet address, please let me know if I should correct it in the initial proposal.

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented Apr 3, 2024

Hi @caaaml ,
thanks for the delivery. The modification on the USDT address needs to be reflected in the initial application, indeed. Please open an amendment PR for this change. Should be quickly approved. :)

@caaaml
Copy link
Contributor Author

caaaml commented Apr 3, 2024

Hey @PieWol,

I've PRed the update to the initial proposal. Please kindly confirm if this has been done correctly :)

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented Apr 10, 2024

Hey @caaaml ,
glad to see the amendment PR was successful. The Milestone delivery has to list the same deliverables as agreed upon in the application. So please add e.g. "testing and testing guide", "license" and other missing deliverables in the tables.

@caaaml
Copy link
Contributor Author

caaaml commented Apr 18, 2024

Hey @PieWol,

Missing deliverables have been added, thanks!

Best,
Kamil

@PieWol PieWol added last milestone The team delivered the last milestone of the project and removed changes requested labels Apr 18, 2024
@PieWol
Copy link
Member

PieWol commented Apr 24, 2024

Hi @caaaml ,
neither the tests for the client nor for the server are passing. I'm using npm testas requested in the readme. Have you checked everything before submitting the milestone? Please check the tests again and let me know once you fixed it. Thanks!

@caaaml
Copy link
Contributor Author

caaaml commented Apr 29, 2024

Hey @PieWol,

Sorry for issues, we've updated the tests - everything should work smooth now!

image
image

Best,
Kamil

@caaaml
Copy link
Contributor Author

caaaml commented May 9, 2024

Hey @PieWol,

What are the next steps to move forward?

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented May 13, 2024

Hi @caaaml ,
your guide for the server doesn't work. Best way to make sure an evaluation is done quickly is to make sure everything is working and the testing guide covers all needed steps for unit testing and running your project locally.

For example your dockerfile doesn't seem to be mentioned at all in the readme or testing guide. Please extend the documentation.

@PieWol
Copy link
Member

PieWol commented May 13, 2024

In case your regular readme isn't designed to cover everything that we expect for a testing guide then please construct a separate testing guide for evaluation purposes.

@PieWol
Copy link
Member

PieWol commented May 13, 2024

I have now put up an evaluation. Please introduce the changes I requested. Thank you!

@PieWol
Copy link
Member

PieWol commented May 21, 2024

@caaaml ,
Hey, hope you are doing well. Any updated from your side?

@caaaml
Copy link
Contributor Author

caaaml commented May 22, 2024

Hey @PieWol,

Sorry for the issues. Our team is still developing this repo for the community and we had to move the deliverables to a different branch (v2.0.3). We've updated all the links, created a testing guide and fixed all that you mentioned in the eval.

Please kindly let us know if everything works fine now!

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented May 26, 2024

Hey @caaaml ,
I checked out your changes which look really promising! Thanks a lot for this well structured update. I will have your evaluation done by tomorrow.

@PieWol
Copy link
Member

PieWol commented May 27, 2024

Hey @caaaml ,
the only thing I could not find which was relevant in the agreement was the possibility to create a pie chart. How can I do this? Otherwise m1 would be accepted.

@caaaml
Copy link
Contributor Author

caaaml commented May 29, 2024

Hey @PieWol,

The repo is updated with the lacking pie chart (please note that links have changed) :) Thanks for the heads up!

The delivery is for both milestones - 1 and 2. Should I open a separate PR for the m2?

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented May 29, 2024

Hey @caaaml , thanks for the swift response! I'll check it out.

We usually recommend to split the delivery in different PRs. In this case though I am confident that the second milestone will be quickly approved after the first one as they are so closely linked together. Just by running the testing guide I already tested both front and backend. Meaning if you are willing to wait for me to finish the second milestone evaluation before you can invoice the funds for the first milestone feel free to leave it the way it is.

@caaaml
Copy link
Contributor Author

caaaml commented May 29, 2024

Hey @PieWol,

Perfect, we'll wait for the m2 to be checked as well!

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented May 30, 2024

Hey @caaaml ,
The first milestone is accepted 🎉
I will evaluate the second milestone shortly.

@PieWol
Copy link
Member

PieWol commented Jun 5, 2024

Hey @caaaml ,
I have put up my evaluation for M2 for you to review and address my comments and questions. Looking forward to your updates.

@caaaml
Copy link
Contributor Author

caaaml commented Jun 6, 2024

Hey @PieWol,

We've updated the repo according to your evaluation.

Regarding the M2 part 3. "Report & estimation" - we discussed this with Sebastian and agreed to limit this part to Changelog only. The estimation was supposed to include the costs for next features to be built with W3F grants but Sebastian said the follow-up should be done through DF grants, not W3F - so the estimation became obsolete.

Let me kindly know if everything works fine now!

Best,
Kamil

@PieWol
Copy link
Member

PieWol commented Jun 8, 2024

Hey @caaaml ,
thanks for the changes. I still had no luck to start the service without docker following the readme. I updated my evaluation and I'm looking forward to your comments. Thanks!

@rrozek
Copy link

rrozek commented Jun 8, 2024

Hi @PieWol , i’m not sure if @caaaml mentioned it, but since there were new commits, the current tag to verify is v2.0.5 (https://github.com/tokenguardio/dashboard-creator-server/tree/v2.0.5)

or you can check dev branch at it is in sync with v2.0.5 now.
(https://github.com/tokenguardio/dashboard-creator-server/tree/dev)

if you can confirm that you checked one of those (that would be surprising) then i’ll take another look at that. Sorry for inconvenience

edit:
Do you mean service does not start or it starts but doesnt work due to missing co-existing components like db-api/creator-client?
If it is the latter (as mentioned in „note”) then i believe it is ok?

cheers,
Kuba

@PieWol
Copy link
Member

PieWol commented Jun 8, 2024

Hey @rrozek ,
I tried it with the updated version. I assumed that .localhost.env contains a valid default config to launch the service.

> [email protected] server:dev
> ts-node-dev --inspect=0.0.0.0:9229 --exit-child --respawn -r tsconfig-paths/register src/server.ts

[INFO] 14:52:36 ts-node-dev ver. 2.0.0 (using ts-node ver. 10.9.2, typescript ver. 4.9.5)
Debugger listening on ws://0.0.0.0:9229/5058b6d5-f7ad-43c4-a296-e837a7464e11
For help, see: https://nodejs.org/en/docs/inspector
2024-06-08 14:52:36.999 info Logger level is set to debug
(node:49961) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
2024-06-08 14:52:37.977 info Connecting to MongoDB without CA cert
2024-06-08 14:53:07.991 error MongoDB connection error: getaddrinfo ENOTFOUND db
2024-06-08 14:53:07.997 error getaddrinfo ENOTFOUND db 
 Error: getaddrinfo ENOTFOUND db
    at Object.connect (/Users/-/Desktop/dashboard-creator/dashboard-creator-server/src/db.ts:28:11)
    at Server.<anonymous> (/Users/-/Desktop/dashboard-creator/dashboard-creator-server/src/server.ts:11:3)
[ERROR] 14:53:07 Error: getaddrinfo ENOTFOUND db
2024-06-08 14:53:07.998 info Server closed

@rrozek
Copy link

rrozek commented Jun 8, 2024

Thanks for providing logs!
So as i suspected the service starts but does not connect to mongodb (because it doesnt exist)

2024-06-08 14:53:07.991 error MongoDB connection error: getaddrinfo ENOTFOUND db

We could do the academic dispute if it means service works or not, but i get your point that it is not operational („it is, by design”)

similar thing is with the frontend that it opens but returns errors if ran without docker-compose.

i believe the instructions are clear enough given the comments about fully-working compose in TESTING dir.

cheers,
Kuba

@PieWol
Copy link
Member

PieWol commented Jun 8, 2024

Right, well I would argue that a "getting started" section should cover all steps necessary to launch the service. If there is the db missing then this should either be fixed so that the readme includes a step to start the db or the section should be discarded. What would you suggest? Do you want to fix it or scrap it? @rrozek

@rrozek
Copy link

rrozek commented Jun 8, 2024

In such case, i believe it makes more sense to add all steps required for potential further development of the service (without the need for the frontend) by future users/developers here.

I’ll update the getting started section to use minimal docker-compose project which i will include in repo root dir „as usual”

ETA: this evening

cheers,
Kuba

@PieWol
Copy link
Member

PieWol commented Jun 8, 2024

Awesome, happy to hear that! Thanks for putting in the extra work :)

@rrozek
Copy link

rrozek commented Jun 9, 2024

hi @PieWol ,
done. https://github.com/tokenguardio/dashboard-creator-server/tree/v2.0.6
or simply https://github.com/tokenguardio/dashboard-creator-server/dev
(please note that default repo branch is different than dev)

thank you!

@PieWol
Copy link
Member

PieWol commented Jun 10, 2024

Thanks, both milestones are now accepted 🎉

@PieWol PieWol merged commit 03e3512 into w3f:master Jun 10, 2024
3 checks passed
Copy link

🪙 Please fill out the invoice form in order to initiate the payment process. Thank you!

@caaaml
Copy link
Contributor Author

caaaml commented Jun 10, 2024

Hey @PieWol,

Thank you as well! Invoice submitted.

Best,
Kamil

@semuelle
Copy link
Member

Hi @caaaml. The invoice you submitted lists a bank account, but the agreement is for USDT payment. Could you update one to match the other? Both would be fine for us, it just needs to match to avoid misunderstandings.

@caaaml
Copy link
Contributor Author

caaaml commented Jun 13, 2024

Hey @semuelle,

I've create another PR but you need to accept and resolve the conflict:
w3f/Grants-Program#2331

Best,
Kamil

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last milestone The team delivered the last milestone of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants