Coder Social home page Coder Social logo

hoodie-account-client's People

Contributors

acdpnk avatar andru avatar capellini avatar christophwitzko avatar dsims00 avatar ederribeiro avatar espy avatar finnp avatar gr2m avatar greenkeeper[bot] avatar hipsterbrown avatar ianstalter123 avatar julianacook avatar jwalter avatar kipply avatar lindsayelia avatar lowprofiledog avatar marisalaneous avatar mbad0la avatar mxstbr avatar nickcolley avatar ransomw avatar sohini-roy avatar thomasjinlo avatar toh82 avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

hoodie-account-client's Issues

admin.sessions.findAll filter by username

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.

Implement account.hasInvalidSession()

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

getState does not set a fallback url

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.

Add options.id to constructor

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"
                    }
                }
            }

Bring code coverage to 100%

current status: [Coverage 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

implement accountAdmin.accounts.removeAll()

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

account.signOut() does not resolve with account properties

This is what we want

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

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

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

README: setup instructions

welcome-to-open-source-atlanta

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 :)

Add tests for account.get & account.profile.get when not signed in

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

use `state.url` instead of `state.baseUrl` in fetch.js and profile-fetch.js

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

implement accountAdmin.accounts.updateAll()

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

bind null in object construction

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.

trigger "update" event when account.update succeeds

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

Implement accountAdmin events

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)

account.signIn() to resolve with {id, account}, but resolves with {id, username}

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

admin.accounts.find/update/remove(object)

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]

Implement account.signUp with profile: {...} option

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

account.update: set session ID from `x-set-session` response header

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


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

handle unexpected 404s in utils/request.js

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.

hoodie.account.fetch(), .update(), .profile.fetch() and .profile.update() should fail with UnauthenticatedError if session is invalid, but currently fails with "Not FoundError"

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

Implement account.validate() method

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

Migrate to new Architecture

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

renew session after changing username / password

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.


implement account.update()

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

hooks for signin & signout

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

Implement account.request

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

Examples

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

README: example for account.profile.fetch() has a bug

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 :)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.