hoodiehq / hoodie-account-client Goto Github PK

This issue is reserved for participants of Welcome to Open Source, Atlanta.
If it’s still available after April 2nd, it’s all yours :)

🎯 The Goal

Currently, testing section of this repository’s README has no instructions on how to setup it up locally for testing. The steps are the same as for hoodie-client, only the URL is different as it’s a different repository

📋 Step by Step

If this is your first, welcome 🎉 😄 Here is a great tutorial on how to send a pull request

• claim this issue (comment below)
• Clone the repository to your computer
• Edit the README.md file.
• Commit the change docs(README): setup instructions
• Start a Pull Request. Mention closes #88 in the description of the pull request
• Done 👍 Comment below to ask for a review :)

Ping us in the Hoodie Chat or on Twitter if you have any questions :)

follow up for #7

accountAdmin.accounts.updateAll() is a bit tricky, because at this point, our JSON API has no way to update multiple resources at once, so the only way to implement this would by sending a separate PATCH request for each account. Which is doable, I'd prefer to think about how to provide a REST API for it. The JSON API Bulk Extension could be exactly what we need here

Find, update, and remove are documented as accepting an object with an id, username, or token property, but currently only work when passed a string id.
When passed an object they mistakenly try to GET http://localhost:4200/hoodie/account/api/accounts/[object%20Object]

The idea is that by default, account.validate() always resolves. A custom validation method can be passed as options.validate to the Account constructor.

We assume that state.session is an object here: https://github.com/hoodiehq/hoodie-client-account/blob/master/lib/fetch.js#L14

But when signed out, state.session is undefined

current status: [](https://coveralls.io/github/hoodiehq/hoodie-client-account?branch=master)

npm run test:coverage

...
=============================== Coverage summary ===============================
Statements   : 97.09% ( 400/412 )
Branches     : 91.2% ( 114/125 )
Functions    : 92.86% ( 65/70 )
Lines        : 97.09% ( 400/412 )
================================================================================

• claim this issue (@chrifpa)
• setup the repo locally and make sure that all tests pass when you run npm test
• get a coverage report by running npm run test:coverage. That also creates a coverage folder. Open coverage/lcov-report/index.html for a nice UI that shows you exactly what lines / branches etc are not yet covered
• add a test to fix one of the not covered lines / branches. Commit with WIP #66 coverage
• start a pull request #78
• write tests until 100% coverage is reached, just keep commiting all changes, don’t worry the amount of commits, we will clean them up before merging. Comment below when you are ready for review :)

Ping us in the Hoodie Chat or on Twitter if you have ​​_any_​​ questions

in the signIn method, we miss a return here:
https://github.com/hoodiehq/hoodie-client-account/blob/master/lib/signin.js#L11

At the moment the code execution is continued and the request to the server is sent, even when username & password is not passed.

Tricky one to test, but would be good if we could add that, too.

similar to what we done here: hoodiehq/hoodie-client#14, but without the .trigger method, because there is currently no use case for it

follow up for #6

maybe nock can to this today, otherwise we can try http://sinonjs.org/docs/#server

This is what we want

// given user is signed in as "pat"
account.signOut()

.then(function (properties) {
  properties.username // should be "pat"
})

• claim this issue (@mxstbr)
• start a pull request (#51)
• uncomment failing test and set t.plan(10) to t.plan(11). Test should fail now
• fix the issue
• review & merge
• update gr2m/milestones#88
• uncomment test in Something Tracker App

Ping us in the Hoodie Chat or on Twitter if you have any questions

followup for hoodiehq/hoodie#282

pass created at to the sign up request, so the server can store createdAt and a separate signedUpAt timestamp. Related server issue: hoodiehq/hoodie-account-server#154

account.request(type/*, options*/) // returns promise

Examples

account.request('upgrade', {plan: 'pro', token: 'stripe123'})
.then(function () {
  alert('Upgraded successfully!')
})
.catch(function (error) {
  alert(error)
})

Reported in: #61
When reproducing the Error from #54 a GET Request to http://127.0.0.1:8080/undefined/session/account is send.

Fix that by using state.url instead of state.baseUrl

• claim this issue (@Endi1)
• start a PR
• uncomment integration test test/integration/fetch-url-test.js
• fix state.baseUrl in fetch.js
• fix state.baseUrl in profile-fetch.js
• search for more appearance of state.baseUrl
• run tests and make sure that all are green
• review & merge
• check if #62 is resolved

If a session gets invalidated, "unauthenticate" events get triggered, for example when account.fetch() is called. Once unauthenticated, account.isUnauthenticated shall return true, otherwise false. The unauthenticate state shall also be persisted in localStorage.

account.signIn with the same username creates a new valid session and emits the reauthenticate event

• claim this issue (comment below or assign yourself) (@toh82)
• start a PR (#72)
• add unit tests for account.hasInvalidSession()
• implement account.hasInvalidSession(), and persist the unauthenticate state in localStorage
• update README (add account.hasInvalidSession and remove issue reference in Events)
• review & merge
• update gr2m/milestones#88

• What are sessions? What do I use them for?
• What are admin requests?
• What are tokens?
• What are roles?

e.g. if I try to account.signIn(options) I get the Error: Please include a JSON API response to deserialise error

follow up for hoodiehq/hoodie#357

follow up for https://github.com/hoodiehq/hoodie-account-client#accountsignin. This will need a change in account server

We broke all the routes: https://github.com/hoodiehq/hapi-couchdb-account-api

It's now all JSON API, following the specification at http://docs.accountrestapi.apiary.io

We need to adapt the tests & implementation of the client accordingly

follow up for discussion at hoodiehq/hoodie-client#42 (comment). It’s worth reading to get some background what we try to achieve and why.

so what we want is this

hoodie.account.on('signin', function (account, options) {
  options.hooks.push(function () {
    return Promise.reject(new Error ('foo'))
  })
})
hoodie.account.signIn({username: 'pat', password: 'secert')
.catch(function (error) {
  // fails with "foo" error
})

@danreeves came up with the idea and made a reference implementation of hooks option here:
hoodiehq/hoodie-client@master...danreeves:hoodie-reset

• claim this issue (@gr2m)
• start a PR #76
• add integration tests for before:signin / after:signin events
• implement before:signin / after:signin events
• add integration tests for before:signout / after:signout events
• implement before:signout / after:signout events
• review (@danreeves)
• merge
• update gr2m/milestones#88

• admin.accounts.add
• admin.accounts.find
• admin.accounts.findAll
• admin.accounts.update
• admin.accounts.updateAll ☞ #30
• admin.accounts.remove
• admin.accounts.removeAll ☞ #31

Only the valid session is required for authorization at this point.

• claim this issue (@janpawellek)
• update the README: add account.destroy (it is similar to account.signOut for now)
• start a PR #68
• add unit & integration test. In the latter, make sure that both a signOut and destroy event is triggered
• implement account.destroy()
• review & merge
• update gr2m/milestones#88

• admin.account()
• admin.account().profile.find()
• admin.account().profile.update()
• admin.account().tokens.add()
• admin.account().tokens.find()
• admin.account().tokens.findAll()
• admin.account().tokens.remove()
• admin.account().roles.add()
• admin.account().roles.findAll()
• admin.account().roles.remove()

follow up for #19 / #77

• update the admin.sessions.findAll README (if necessary) to describe how the http api filter query string may be abstracted into the javascript api in order to to filter findAll results on the server side by username
• add some integration tests in a new file matching tests/integration/admin-*.js according to the linked README.
• update the implementation in admin/lib/accounts/find-all.js (if necessary) such that tests pass.
• update the README again (if necessary) to reflect working state of API based on the knowledge gained during implementation

for this issue, tests need not be complete spec coverage, but while making the edits described above, it might be a good time to get more complete spec coverage in integration tests for findAll. an integration test for the filter by username usage is the bare minimum.

after these edits, make edits to the implementation of admin.sessions.add according to the TODO in admin/lib/sessions/add.js, either as part of this or a new issue.

As described in https://github.com/hoodiehq/hoodie-client-account#accountusername

See how hoodie.id is implemented in hoodie-client for comparison

• start a pull request
• write tests for account.username
• implement account.username
• write tests for admin.username
• implement admin.username
• review & merge

account.isSignedIn is document, but not listed in https://github.com/hoodiehq/hoodie-client-account#api

• claim this issue (comment below or assign yourself)
• start a PR
• add the link to the README
• review & merge

Methods

• admin.on()
• admin.one()
• admin.off()
• admin.accounts.on()
• admin.accounts.one()
• admin.accounts.off()
• admin.requests.on()
• admin.requests.one()
• admin.requests.off()

Local events

for admin

• signin
• signout
• unauthenticate
• reauthenticate

for admin.accounts and admin.requests

• change (eventName, properties)
• add (properties)
• update (properties)
• remove (properties)

Note that these events are only triggered when calling the .add / .update / .updateAll / .remove / removeAll methods. They are not triggered data gets changed on the server by someone else (see Remote Events below)

Remote

• trigger change /add /update /remove events for changes on remote (not caused by me)

Find docs here: admin.sessions.add(options)

• claim this issue (@ransomw)
• Add an integration admin-sessions-test.js here: https://github.com/hoodiehq/hoodie-client-account/tree/master/test/integration with a test for admin.sessions.add(options)
• Implement sessions.add method
• update README
• review & merge

Before a new version of account-client gets released, docs are generated automatically from source code, which currently looks like this:

To make it look better, take https://github.com/hoodiehq/pouchdb-hoodie-api as example. We need to add JSDoc Comments like these: https://github.com/hoodiehq/pouchdb-hoodie-api/blob/master/find-or-add.js#L8-L16

To simulate an unauthenticated state, replace session.id in localStorage.account with a bogus value, then reload the page and do account.fetch()

• claim this issue (@Realtin)
• start a PR (#62)
• add unit and integration tests for account.fetch(). In the latter, make sure that an "unauthenticate" event gets triggered
• fix account.fetch()
• repeat steps for account.update, account.profile.fetch and account.profile.update
• review & merge
• update gr2m/milestones#88

https://travis-ci.org/hoodiehq/hoodie-client-account#L833 @boennemann

follow up for #78 (comment)

account.fetch('fullname').then(function (createdAt) {
  alert('Hey there ' + fullname)
})

should be

account.fetch('fullname').then(function (fullname) {
  alert('Hey there ' + fullname)
})

Please help free to turn this into a starter issue <3 Edit my issue description directly, or create a follow up issue so we can close this one :)

Only username and password can be changed at this point, and only the valid session is required for authorization at this point.

• claim this issue (@finnp)
• update the README: add account.update (it is similar to account.profile.update for now)
• add unit & integration test. In the latter, make sure account.username is set to the new username if it got changed, and sign out & back in with the new username / password
• implement account.update()
• review & merge
• update gr2m/milestones#88

e.g. if a user tries to update profile, but the server responds with 403

currently we store the user session in _session, it’s hardcoded here & here & here.

I'd like to make this configurable. My main reason is that I'd like to reuse code between the Account and AccountAdmin, and I'd like to prevent conflicts between the two.

• start PR #24
• update README.md
• update admin/README.md
• implement test
• implement feature
• review & merge

We need support a client-generated ID for our account for Hoodie, as the account ID will be set to hoodie.id, and hoodie.id gets generated upon first initialisation. JSON API explicitly supports client-generated resource IDs

So what we want is this

var account = new Account({
  url: 'https://example.com/api'
  id: 'abc4567'
}) 

account.signUp({
  username: 'pat',
  password: 'secret'
})

to send this request

### Sign Up [PUT /session/account]
+ Request (application/vnd.api+json)

    + Headers

            Accept: application/vnd.api+json

    + Body

            {
                "data": {
                    "type": "account",
                    "id": "abc4567",
                    "attributes": {
                        "username": "john-doe",
                        "password": "secret"
                    }
                }
            }

depends on #56

If account.isUnauthenticated() returns true and the user signs in with the same username, then an reauthenticate event should be triggered (instead of signin)

• claim this issue (@espy)
• start a PR (#60)
• add unit and integration tests. Integration tests can be added here
• implement the reauthenticate event
• update README
• review & merge
• update gr2m/milestones#88

If user is not signed in, account.get() should return undefined. So should account.profile.get(). I think this is already the case, but we don’t have a test for that yet.

Tests need to be added to https://github.com/hoodiehq/hoodie-client-account/blob/master/tests/specs/get.js & https://github.com/hoodiehq/hoodie-client-account/blob/master/tests/specs/get-profile.js. Pass in {} as state, instead of the state variable, and make sure the get methods return undefined

Currently, a user can not sign up and pass profile properties. This will fail

account.signUp({
  username: 'pat',
  password: 'secret',
  profile: {
    fullname: 'Dr Pat Hook'
  }
})

The reason is that JSON API currently does not allow for multiple actions in a single request

So we need to send a separate request to PATCH /session/account/profile
As signUp (creating account) is separated from signIn (creating session), we can't send a Authorization: Bearer ... token. I would suggest we allow to authenticate traditionally with Authoriziation: Basic ... as well, so we would send headers.authorization = 'Basic ' + base64encode('pat:secret')

Authentication with Basic ... and also Token ... will be required for the admin client, too: https://github.com/hoodiehq/hoodie-client-account/tree/master/admin

When reproducing the Error from #54 a GET Request to http://127.0.0.1:8080/undefined/session/account is send.

setting an empty String as a fallback in utils/get-state.js should fix that.

account.update(options) is implemented, but does not yet emit an update event.

• claim this issue (comment below or assign yourself) (@HipsterBrown)
• add update event to README. Commit it with docs(README): update event
• start a PR
• update unit tests at test/unit/update-test.js to check if update event is emitted
• update integration test at test/integration/events-test.js to test for update event
• commit with test: update event & push
• implement the update event and commit with feat: update event & push
• review & merge
• update gr2m/milestones#88

follow up for #7

accountAdmin.accounts.updateAll() is a bit tricky, because at this point, our JSON API has no way to update multiple resources at once, so the only way to implement this would by sending a separate DELETE request for each account. Which is doable, I'd prefer to think about how to provide a REST API for it. The JSON API Bulk Extension could be exactly what we need here

• admin.requests.add
• admin.requests.find
• admin.requests.findAll
• admin.requests.remove
• admin.requests.removeAll

in utils/request.js, we expect response.body to be an object with a errors array property. In some circumstances there might be an issue with that, and we should add a check if response.body.errors exists and if it doesn’t, response.body can be a string like Not found: /hoodie/account/api/session/account.

part of hoodiehq/hoodie#458

• rename to hoodie-account-client
• in package.json, set "name" to "@hoodie/account-client", and add "publishConfig": {"access": "public"}
• find hoodie-account-client and replace with hoodie-account-client
• do commits as in hoodiehq/hoodie-log#31

Once merged

• deprecate hoodie-client-account in the npm registry
• enable coveralls for new repo name (run COVERALLS_REPO_TOKEN=... npm run coverage, then on coveralls.io require 100% coverage for success)
• update branch protection: re-enable coveralls

/cc @gr2m

account.signIn({username: 'pat', password: 'secret'})

currently resolves iwth

{
  sessionId: "287f3a64-df67-4384-a00d-35d0c257f798"
  username: "pat"
}

But should resolve with

{
  id: "287f3a64-df67-4384-a00d-35d0c257f798"
  username: {
    id: "account123",
    username: "pat",
    roles: []
  }
}

As showin in the README: https://github.com/hoodiehq/hoodie-client-account#accountsignin

I guess the way we serialise at the moment is not up to date with the latest spec, so other methods / tests might be needed to be adapted as well

• start PR
• fix test https://github.com/hoodiehq/hoodie-client-account/blob/master/tests/specs/signin-test.js#L87-L90
• fix implementation
• review & merge

follow up for #19 / #77

replace bind(this, with bind(null, per comments in admin/index.js.

also consider updating object construction elsewhere in the repository with the new pattern if discussion deems appropriate.

follow up for #74 (comment)

Right now, we send an internal PUT /session request after a PATCH /session/account request which changed the account username and/or password.

Instead, we want the PATCH /session/account response to optionally include a x-set-session header which would include the new session id, so that it can be updated without sending an extra request

Working on your first Pull Request? You can learn how from this free series How to Contribute to an Open Source Project on GitHub

• claim this issue (@gr2m)
• Setup repository locally and make sure that all tests pass
• revert the changes of https://github.com/hoodiehq/hoodie-client-account/pull/74/files
• update test/integration/update-account-test.js#L19: set a reply header for the PATCH /session/account response to include the x-set-session header with a new session id.
• add request headers to the DELETE /session request mock. Make sure the new session id is passed in the Authorization: bearer <new session id here> header. Make sure test fails when you run npm test
• commit all test files with test(update): session id
• fix the implementation in lib/accounts/update.js. We might need to change utils/request.js to resolve with both the response body & headers instead of just the body. Make sure tests pass when you run npm test
• commit all changes with fix(update): session id
• pull request: #94
• Someone else: review & merge

Ping us in the Hoodie Chat or on Twitter if you have any questions :)

account.update(options) is implemented, but the problem is that once the username and/or password changes, the session ID gets invalidated.

So what we need is to renew the session with the new username / password by doing a PUT /session request, but without triggering any extra events.

• claim this issue (@NickColley)
• update integration test at test/integration/update-account-test.js to test that account.get('session.id') and account.username has been updated
• commit with test: renew session after username / password change
• start a PR (#74)
• implement the session update
• commit with fix: renew session after username / password change & push
• review & merge
• update gr2m/milestones#88
• uncomment tests in app-something-tracker to make sure the session renewal works

follow up for hoodiehq/hoodie#262