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

Hoodie Account Server: replace options.usersDb with options.PouchDB #52

Closed
10 tasks
LowProfileDog opened this issue Oct 5, 2016 · 15 comments · Fixed by hoodiehq/hoodie-account-server-api#2
Closed
10 tasks
Assignees
Labels

Comments

@LowProfileDog
Copy link
Collaborator

LowProfileDog commented Oct 5, 2016

🎃💻👕 Hacktoberfest: Trick or Treat!

If you haven’t yet, sign up for Hacktoberfest to earn an exclusive T-Shirt. Plus I’m sure we can teach you a cool trick or two in the process

🤔 What you will need to know

You should be experienced with JavaScript, Node.js and testing. Some knowledge about hapi.js will help, but won’t be required.

❓ The Motivation

Hoodie is very modular. Currently the options passed to the server modules are not coherent. For example, hoodie accepts options.db, hoodie-server accepts options.PouchDB and hoodie-account-server accepts options.usersDb. We want to normalise these options across all server modules.

🎯 The goal

The server modules all interact with a database in one way or another, for which we use PouchDB. Currently, hoodie-account-server accepts options.usersDb which is a PouchDB instance, and then internally gets PouchDB from its constructor at plugin/index.js#L23. What we want instead is that the register functions defined in plugin/index.js accepts options.PouchDB, and the users database should be initialised in there instead of outside of the plugin.

📋 Step by Step

  • 🙋 Claim this issue: Comment below (or assign yourself and continue at step 4 :)
    Please 🙏 only claim if you want to start working on it within a day.
    Once claimed we add you as contributor to this repository.
  • 👌 Accept our invitation to this repository. Once accepted, assign yourself to this repository
  • 👓 Please review our Code of Conduct
    In a nutshell: be patient and actively kind with us 😊
  • 🔄 replace the up for grabs label with in progress.
  • If you are an existing Hoodie Contributor, create a new branch. If you are a new contributor, welcome 👋 Please fork hoodie-account-server.
    Setup the repository locally as described in tests/README.md (change the repository URL to your fork). Ensure the tests are all passing.
  • Write an integration test (it’s currently missing, ooops) to test plugin/index.js, make sure it passes
  • 🔀 Start a Pull Request. Mention closes hoodiehq/camp#52 in the description.
    If this is your first, welcome 🎉 😄 Here is a great tutorial on how to send a pull request using the terminal.
  • Adapt the test for the new API, tests should now fail
  • Make the necessary changes in the code to make the test pass. ProTip:tm:: you can run a single test with node_modules/.bin/tap tests/integration/api/accounts-test.js
  • 🏁 Done 👍 Once you think your pull request is ready for a first review, replace the in progress label with ready. Ask in comments for a review :)

🤔❓ Questions

Ping us in the Hoodie Chat or on Twitter

@vgvenkat
Copy link

vgvenkat commented Oct 5, 2016

Hey! this is my first contribution. Can I have a try?

@LowProfileDog
Copy link
Collaborator Author

sure! We invited you to this repository, you can accept the invitation here: https://github.com/hoodiehq/camp/invitations

Once accepted, you can assign yourself and work off the check boxes. Let us know if you get stuck, or if you realise that it’s too hard after all, no problem :) good luck!

@vgvenkat vgvenkat self-assigned this Oct 5, 2016
@vgvenkat
Copy link

vgvenkat commented Oct 8, 2016

I am opening up again for some one to grab this one. I need to work on a few more things before I can continue this issue. Open for any one!

@vgvenkat vgvenkat removed their assignment Oct 8, 2016
@Taekyoon
Copy link
Collaborator

Taekyoon commented Oct 10, 2016

Can I try this one?

@gr2m
Copy link
Member

gr2m commented Oct 10, 2016

Maybe give it a try and let us know how it goes?

@Taekyoon
Copy link
Collaborator

Sure~! I'll do that!

@Taekyoon
Copy link
Collaborator

Actually, I have no idea what to do....
But I tried this.
options.PouchDB = require('pouchdb-admins') options.userDb.constructor.plugin(options.PouchDB)
instead
options.userDb.constructor.plugin(require('pouchdb-admins'))
Can you give me some hints to do this things?

@gr2m
Copy link
Member

gr2m commented Oct 10, 2016

I’m very sorry, I wish I could coach you trough this issue but I don’t have the time right now :( This issue really requires quite a bit of JavaScript / Node.js knowledge, it’s not well suited for beginners. But keep your eyes open, we plan to create more beginner-friendly issues over the next weeks

@Taekyoon
Copy link
Collaborator

So, now I'm trying to understand Node.js. actually, I have some knowledge about JS, but not Node.js. If I read documents of Node.js, where should I concentrate to contribute this issue?
I had a practice on tutorial of Django, so I would work if it is related. Just give me some keywords to figure it.

@gr2m
Copy link
Member

gr2m commented Oct 10, 2016

For this issue, you need to have good experience with Node.js, testing and code refactoring. You will also need to make yourself familiar with http://hapijs.com/ and https://pouchdb.com/. I’d start here: http://exercism.io/languages/javascript

@Taekyoon
Copy link
Collaborator

Okay I'll try it first and after if this issue is not suited for me, I'll lend it to the other contributors.

@gr2m gr2m self-assigned this Oct 15, 2016
@gr2m
Copy link
Member

gr2m commented Oct 15, 2016

I assigned it to me because we are doing some changes that will affect this issue, I’ll free it up if it’s still applicable after the change

@Taekyoon
Copy link
Collaborator

I think that's fine because I cannot handle with this right now.

@gr2m
Copy link
Member

gr2m commented Oct 15, 2016

@Taekyoon I did it here if you want to check it out: hoodiehq/hoodie-account-server-api#2

@Taekyoon
Copy link
Collaborator

Thanks! That would be helpful to study!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants