Change password form #271
Conversation
|
@rhodges @pollardld I added you two on this PR for visibility, but I feel ok enough about this change that your review is not required. |
There was a problem hiding this comment.
Pull request overview
Adds a user-facing “Change Password” flow using a Bootstrap modal + AJAX, backed by a Django PasswordChangeView, and removes legacy/unused login/registration templates.
Changes:
- Add
TEKDBPasswordChangeViewendpoint (/change_password/) returning JSON for success/errors. - Add “Change Password” modal + client-side AJAX submission/error rendering.
- Remove unused login/registration/forgot templates and dead view code; add login view tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| TEKDB/login/views.py | Introduces JSON-based password-change view and removes unused forgot view code. |
| TEKDB/login/tests/test_views.py | Adds tests covering login endpoints and password-change view helpers. |
| TEKDB/login/templates/registration/registration_form.html | Removes legacy registration template. |
| TEKDB/login/templates/registration/login.html | Removes dead/commented markup. |
| TEKDB/login/templates/forgot.html | Removes unused forgot-password template. |
| TEKDB/login/templates/create.html | Removes unused template. |
| TEKDB/login/templates/change_password.html | Adds the password change form markup used in the modal. |
| TEKDB/login/static/login/js/account.js | Adds AJAX helper for password changes. |
| TEKDB/explore/templates/navbar.html | Fixes href typo and adds “Change Password” menu item. |
| TEKDB/explore/templates/modals.html | Adds “Change Password” modal content and updates modal close buttons. |
| TEKDB/explore/static/explore/js/modals.js | Wires up modal submit handling, success message, and error rendering. |
| TEKDB/explore/static/explore/css/forms.css | Removes registration helptext CSS rule. |
| TEKDB/TEKDB/urls.py | Adds /change_password/ route. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return JsonResponse(context) | ||
|
|
||
|
|
||
| class TEKDBPasswordChangeView(PasswordChangeView): |
There was a problem hiding this comment.
TEKDBPasswordChangeView inherits PasswordChangeView but doesn't set template_name and the repo doesn't appear to have Django's default registration/password_change_form.html. A GET to /change_password/ will likely raise TemplateDoesNotExist. Either provide/point to a template, or explicitly disallow GET (e.g., only allow POST and return 405 for others).
| class TEKDBPasswordChangeView(PasswordChangeView): | |
| class TEKDBPasswordChangeView(PasswordChangeView): | |
| http_method_names = ["post"] |
There was a problem hiding this comment.
We are using a modal so we don't need a template file
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| }, | ||
| error: function(response) { | ||
| console.log('%cerror with change password request submission: %o', 'color: red', response.data); |
There was a problem hiding this comment.
In the AJAX error callback, response.data is not a standard jQuery jqXHR field (it will typically be undefined). This makes the console message misleading and can hide the actual server response. Log response.responseJSON / response.responseText instead.
| console.log('%cerror with change password request submission: %o', 'color: red', response.data); | |
| console.log('%cerror with change password request submission: %o', 'color: red', response.responseJSON || response.responseText); |
| class TEKDBPasswordChangeView(PasswordChangeView): | ||
| def form_invalid(self, form): | ||
| return JsonResponse({"data": form.errors, "success": False}, status=400) | ||
|
|
||
| def form_valid(self, form): | ||
| self.object = form.save() | ||
| # prevent user’s auth session to be invalidated | ||
| # and user have to log in again after password change | ||
| update_session_auth_hash(self.request, self.object) | ||
| return JsonResponse({"data": None, "success": True}, status=200) |
There was a problem hiding this comment.
TEKDBPasswordChangeView is mounted at /change_password/ and still supports GET via PasswordChangeView. Since the project doesn’t have registration/password_change_form.html, a GET request will raise TemplateDoesNotExist (500). If this endpoint is intended to be AJAX-only, explicitly disallow GET (e.g., http_method_names = ['post'] or implement get() to return 405). Alternatively, set template_name to an existing template if GET should render a page.
There was a problem hiding this comment.
if the user navigates to /change_password/ the default django form for changing the password renders. Submission of this form returns a json of {data: ... , success: ... }. Not a great UX but it is functional and is unlikely for a user to discover the route.
| const errorLists = document.querySelectorAll("#changePasswordModal ul"); | ||
| if (errorLists.length) { | ||
| errorLists.forEach((list) => list.remove()); | ||
| } |
There was a problem hiding this comment.
On successful password change you remove <ul> error lists, but you don’t clear the red border-color styling that was applied to invalid inputs. If the user fixes errors and then succeeds without closing the modal, the inputs can remain styled as invalid. Consider clearing the input error styles in the success branch (similar to the hidden.bs.modal cleanup).
| } | |
| } | |
| // reset input border color | |
| const passwordInputs = document.querySelectorAll("#changePasswordModal input[type='password']"); | |
| passwordInputs.forEach((input) => input.setAttribute("style", "")); |
| // display error messages | ||
| for (const elementId in response.data.data) { | ||
| const errorList = document.createElement("ul"); | ||
| const erroredInput = document.querySelector(`#${elementId}`); | ||
| erroredInput.after(errorList); | ||
| erroredInput.setAttribute("style", "border-color: red;"); |
There was a problem hiding this comment.
When handling a failed submission, the code appends new <ul> error lists after each input but doesn’t clear any existing error lists first. Submitting the form multiple times with errors will duplicate error messages in the modal. Clear prior error lists (and potentially reset input styles) before rendering the new errors.
pollardld
left a comment
There was a problem hiding this comment.
I seconded one of Copilot's comments regarding replacing a submit event handler, as opposed to adding each time a modal is opening.
Other than that, looks great!
asana task
Description
Adds a form for a user to change their own password. The form requires the user to enter their old password, and the new password twice. This is the format that
PasswordChangeViewexpects. The form displays a success message, and errors. I added a "close" button to the login modal to make this option more clear, rather than just clicking out of the modal.I also deleted some code in the
loginmodule that is not used anymore.Screenshots
The login modal with the "close" button

Change Password button in the nav

Default form state

Successful form submission

Form with errors
