| Summary: | Patron->store falls back to stored password when modifying patron | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
| Component: | Architecture, internals, and plumbing | Assignee: | Bugs List <koha-bugs> |
| Status: | NEW --- | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | jonathan.druart, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 21087 | ||
| Bug Blocks: | |||
Coming from bug 21087. I don't have time right now to deep dive into this but maybe the answer is there? (In reply to Jonathan Druart from comment #1) > Coming from bug 21087. I don't have time right now to deep dive into this > but maybe the answer is there? Ah thanks. Yes thats the one: From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Wed, 25 Jul 2018 10:38:48 -0300 Subject: [PATCH] Bug 21087: Do not store the password in ->store for existing patrons When ->store is called on an existing patron we must not save the password, but keep the one in DB instead. There is a dedicated method (update_password) to call when the password need to be updated Signed-off-by: John Doe <you@example.com> Why did John Doe sign that one off? LOL It would be nice to know the reasons why we should not do it. We can always call the dedicated method from store (to perform needed checks etc).. There was no discussion about this in that report btw. |
At some point we added this in patron->store (ModMember-labeled part): # Password must be updated using $self->set_password $self->password($self_from_storage->password); I stumbled over that requirement in a custom script updating specific patron accounts. But we do allow to set the password in the AddMember part. Which feels inconsistent at the least.. But why do we silently(!) ignore a new password here? A warn might have been handy too ;) Couldnt we just add a call to set_password in store and perhaps add a param to not explicitly save it (when coming from store) or something like that?