search

hoodiehq / hoodie-account-client

:dog: Account client API for the browser
https://hoodiehq.github.io/hoodie-account-client
Apache License 2.0
11 stars 24 forks source link

92/store created at #117

Closed thomasjinlo closed 7 years ago

thomasjinlo commented 7 years ago

Not sure exactly where createdAt should be implemented in the account-client. I'm still working on understanding the architecture. During account.signUp(options) should options object already have a property createdAt? If so where is this options object being passed in from?

Maybe my whole approach might be wrong, feedback is much appreciated :)

thomasjinlo commented 7 years ago

After doing some more exploring I came to the conclusion that during the process of getting the state, if storedAccount returns null, we should add createdAt to the state.account property. implementation

Also have to pass createdAt to serialize and set it as a `data' property. implementation

Please let me know if this is not the correct approach.

thomasjinlo commented 7 years ago

After making the necessary changes here, I noticed the tests were failing.

It seems for the integration/sign-up-with-id.js I just had to move the clock above creation of new Account, so that it will start the clock before get-state is run. commit

For the unit/sign-up-test.js it was a little more tricky, and I'm not sure if it's the correct approach. I noticed get-state was never called and state was being created in the test, with some stubs. My approach was to set account.createdAt with date, so when signUp is called it'll handle the process correctly.

gr2m commented 7 years ago

It seems for the integration/sign-up-with-id.js I just had to move the clock above creation of new Account, so that it will start the clock before get-state is run.

Yes, that makes sense, that we the Date object is mocked before the new Date() in the code

For the unit/sign-up-test.js it was a little more tricky, and I'm not sure if it's the correct approach. I noticed get-state was never called and state was being created in the test, with some stubs. My approach was to set account.createdAt with date, so when signUp is called it'll handle the process correctly.

This is exactly right. In general, the idea of unit/* tests is to only test one "unit", usually meaning one file, and mocking everything else. And you mocked the state which is exactly right, and also the reason why we code Hoodie in a functional style where we implicitly pass in state to API methods: it makes testing them very easy :)

This looks great, I’ll have a final look trough the code and test it locally, and if all is good, let’s ship it :shipit:

gr2m commented 7 years ago

great work @thomasjinlo 👏