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

[DISCUSSION] feat: allow server-side modification of user doc on account creation #143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janl
Copy link
Member

@janl janl commented Apr 3, 2016

Heya, I’m using this outside of Hoodie and I found the need to be able to change the user doc on account creation.

E.g. I need to add another role to the user doc, and the server will know which one.

My idea was to add a function, for illustration purposes called userDocModificationFun (please bikeshed), to pass into the plugin options, and then, if that function exists, it gets called on account.add(). In order to support this, the top level options object needs to be passed down to account.add() (which looks like was planned anyway, this adds the plumbing).

Please let me know if this is an acceptable addition. If yes, I’m happy to add tests and clean things up.

@gr2m
Copy link
Member

gr2m commented Apr 3, 2016

When you use hoodie-account-server outside Hoodie, can you explain why you can’t modify what you pass to server.plugins.account.api.account.add(options) directly?

@janl
Copy link
Member Author

janl commented Apr 4, 2016

@gr2m I’m using the hoodie-account-client as well.

@janl
Copy link
Member Author

janl commented Apr 4, 2016

I could pass these things in from before hoodie-account-client, but I can’t trust the client about these things.

@gr2m
Copy link
Member

gr2m commented Apr 4, 2016

Ah, what you need is a hook like pre:add where you can alter the account object before it gets stored to the users database? We have something similar for the client API: https://github.com/hoodiehq/hoodie-client/blob/issue/3/lib/init.js#L6-L10 It's currently undocumented because we are not 100% happy with the API, we could do something similar for the server side, too.

One idea would be to see how hapi is doing it with their request lifecycle. We also had a discussion about a hook dream API here: hoodiehq/hoodie-client#42, which might work for your use case as well.

tl;dr;

  • the topic is relevant
  • we don’t have a simple solution
  • we can add an undocumented API for the time being so you can move forward

@gr2m
Copy link
Member

gr2m commented Apr 4, 2016

just an idea for a workaround, you could maybe use https://github.com/pouchdb/pouchdb-wrappers to wrap the PouchDB instance that you pass to hoodie-account-server?

@janl
Copy link
Member Author

janl commented Apr 4, 2016

I like the idea of hooks and other things, but I’ve now realised, that the information I’m storing is only available at request-time (e.g. request.headers.host), so I need to figure out how to be able to access that, or pass that down. Will reopen, when I’ve got something.

@janl janl closed this Apr 4, 2016
@janl janl removed the in progress label Apr 4, 2016
@janl janl reopened this Apr 4, 2016
@janl janl force-pushed the server-profile-hook branch from 049056c to dc69e68 Compare April 4, 2016 20:21
@janl
Copy link
Member Author

janl commented Apr 4, 2016

well that’s embarrassing 😳 here’s a version that does that :D, so my stuff can be made to work (squashed and force pushed)


if (hooks.account && hooks.account.beforeAdd &&
typeof hooks.account.beforeAdd === 'function') {
doc = hooks.account.beforeAdd(doc, request)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we’d make this a util of course

@janl
Copy link
Member Author

janl commented Aug 19, 2016

The doc I want to augment is created in api/account/add.js, so running the hook before that doesn’t really work. Although I like the idea of such a design. Maybe we can pass a prototype doc into addAccount() that gets merged with the default defined in there?

@gr2m
Copy link
Member

gr2m commented Aug 28, 2016

We had a discussion on this and will see if we can use the custom "signup" route handler for this. This feature is not yet implemented. The idea is that besides custom requests that can be passed as on object to the @hoodie/acocunt-server hapi plugin, you can also pass in account routes for signup, signin, signout and destroy, which would then overwrite the default route handlers from routes/account.js. It will mean some code duplication, but I see this as a power user tool, and the benefit would be minimal added complexity to @hoodie/acocunt-server as it is today

@gr2m
Copy link
Member

gr2m commented Dec 26, 2016

@janl we now use before-after-hook at several places in Hoodie. I think that could fit perfectly with your initial requirement

Heya, I’m using this outside of Hoodie and I found the need to be able to change the user doc on account creation.

E.g. I need to add another role to the user doc, and the server will know which one.

We could for example expose an add hook, which would wrap this db.put(doc) call so that you could do something like this

server.plugins.account.api.hook.before('add', function (properties) {
  properties.roles.push('mycustomrole')
})

server.plugins.account.api is exposed by the @hoodie/account-server hapi plugin, so you can call it after registration is done. We could also add a hooks option to the plugin itself, it could work like this:

server.register({
  register: hapiAccount,
  options: {
    // other options
    hooks: {
      add: {
        before: function (properties) { properties.roles.push('mycustomrole') }
      }
    }
  }
}, function (error) {
  if (error) {
    throw error
  }

  // plugin account api, see below
});

Would that be helpful for your use case?

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

Successfully merging this pull request may close these issues.

2 participants