meteor-accounts-meld
meteor-accounts-meld copied to clipboard
Fix bug #21 via brettle:accounts-{add-service,accounts-multiple}
Fix the "add new password account" case by just doing
api.use('brettle:accounts-add-service')
.
Fix the "meld existing password account" case by copying logic from
updateOrCreateUserFromExternalService()
into a validateSwitch
callback registered with brettle:accounts-multiple
.
Add tests for both cases.
I left the updateOrCreateUserFromExternalService
monkey patch in place to handle the non-password case (as @splendido requested). FWIW, brettle:accounts-multiple
could handle the non-password cases as well so that the monkey patch could be removed. Let me know if you'd be interested in a PR that would do that.
Seems a good work!
So you would get rid also of the updateOrCreateUserFromExternalService
function and use your package instead?
Yes. Interested?
To be honest I'd rather prefer to publish the perfect package under a non-private namespace, be it useraccounts or anything else.
Accepting a PR about this for this package would make it half mine and half not and I'm not sure I'll have the time to keep it up to date and grant its security.
Would you be willing to get access to a repo under meteor-useraccounts to develop useraccounts:multi-account
or similar?
Would useraccounts:multi-account
be the same as (or similar to) brettle:accounts-multi
brettle:accounts-multiple
or would it be something else? I'd be happy to move any/all of the brettle:accounts-*
suite (see brettle:accounts-deluxe
for an overview) under meteor-useraccounts. Doing so would expand the scope of useraccounts (which is primarily UI oriented at the moment) to include lots of packages that don't have any UI component. If you're OK with that, I think it would be great.
I saw you have a number of pacakges, but I had no time yet to check them out... :(
In case we're not talking about a huge number of code lines, wouldn't be it possible to have one single configurable package to rule them all? ;-)
btw, the name useraccounts:multi-account
was just a randomly typed one, any better naming will be accepted ;-)
I prefer one package per feature so that app developers: 1) don't need to worry about configuration (just meteor add feature-package
), 2) have less code to vet and 3) don't get upgraded as a result of changes to unrelated features. Also contributors have less code that they need to understand before contributing, and reuse is encouraged.
All that said, it's still possible to have one configurable package. It would just use the smaller packages and expose the necessary configuration options. This would require some minor changes to the existing smaller packages to support having them disabled by default, but that is doable.
Should we start a discussion about this on the related trello card?
Perhaps also @zimme could be interested in participating ;-)
I've joined in on the conversation on the trello card and I'll keep an eye on this PR too.