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

KAZOO-3669: replace reset password behaviour #1451

Merged
merged 1 commit into from May 2, 2016
Merged

KAZOO-3669: replace reset password behaviour #1451

merged 1 commit into from May 2, 2016

Conversation

fenollp
Copy link
Contributor

@fenollp fenollp commented Nov 19, 2015

Notes

  • needs testing
  • waiting on a UI ticket: UI-1925 UI-1819
  • not sure about the way to crossbar_cleanup
  • why does my vm not send emailsdammit

lager:debug("removed yesterday's expired reset_ids")
end.

ensure_reset_id_deleted([Doc|Docs]) ->
Copy link
Member

Choose a reason for hiding this comment

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

spec

@fenollp
Copy link
Contributor Author

fenollp commented Nov 20, 2015

@jamesaimonetti On using couch_mgr:save_docs/2 instead: I don't think I can save every user's doc doc ?WH_ACCOUNTS_DB, right? Because I need to save each Doc to its own wh_doc:account_db(Doc), right?

,{'limit', couch_util:max_bulk_insert()}
,'include_docs'
],
case couch_mgr:get_results(?WH_ACCOUNTS_DB, ?LIST_BY_MTIME, ViewOptions) of
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused why the view is being looked up in accounts? AFAICT you are saving the reset docs back to the account database (and loading the user doc via reset_id pulls out the account id and uses that account database). Why would cleanup occur by querying the accounts database then? Those are account docs, not user docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are totally right. I just have no idea how I can fetch user documents that were modified yesterday and that have ?RESET_IDundefined?

Copy link
Member

Choose a reason for hiding this comment

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

You set reset_id on the user though; what happens if their doc is updated by an admin? What if they have a script that sets some property every day, bumping the pvt_modified each time (and thus never cleaning up the reset_id).

Could it simplify life if you tracked the reset attempt as its own document?

{"_id":"reset_id_256"
 ,"user_id":"user_id_32"
 ,"pvt_created":timestamp
 ,"pvt_type":"password_reset"
}

Now you can directly fetch the doc instead of using a view, and looking up the user doc is a open_cache_doc/2 call away as well.

Cleanup then becomes binding for crossbar_cleanup:binding_account() and cleanup_reset_ids(AccountDb) can query maintenance/listing_by_type with {'key', <<"password_reset">>}. I would wager this result set will typically be small enough that iterating over it and checking pvt_modified won't be overly expensive in memory/compute power.

Thoughts?

@jamesaimonetti
Copy link
Member

@fenollp you have conflicts here now. Resolve so I can merge it!

@fenollp
Copy link
Contributor Author

fenollp commented Dec 29, 2015

@jamesaimonetti please make sure this works on your side too

@mark2600 mark2600 merged commit 4d9aa9d into master May 2, 2016
@mark2600 mark2600 deleted the KAZOO-3669 branch May 2, 2016 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants