Merged
Conversation
The tom_eso urlpatterns are namespaced in tom_eso_urls.py. Since the facility template is being rendered by tom_observations.views.ObservationCreateView, tom_observations is the app whose urls are searched for the viewname. Since they exist in tom_eso.urls, not tom_observatons.urls, we must provide the specific namespace to look for them in.
There was a problem hiding this comment.
Pull Request Overview
This PR integrates new update and encryption handling features for the ESO Facility. Key changes include:
- Adding the ProfileUpdateView class to enable user updates on ESO profiles.
- Updating URL patterns and htmx endpoint references to use namespaced paths.
- Introducing integration points in apps.py, extending model encryption logic, and adding corresponding tests and templates.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tom_eso/views.py | Adds a new UpdateView for updating user ESO profiles and handles form encryption keys. |
| tom_eso/urls.py | Updates URL patterns to use namespaced view references and includes profile update URL. |
| tom_eso/tests.py | Introduces tests for the custom encrypted binary field behavior. |
| tom_eso/templatetags/eso_extras.py | Provides an inclusion tag to expose ESO profile data to templates. |
| tom_eso/templates/tom_eso/partials/eso_user_profile.html | Adds a new partial template for displaying ESO profile information. |
| tom_eso/templates/tom_eso/eso_update_user_profile.html | Introduces a simple form template for updating ESO profiles. |
| tom_eso/models.py | Implements an EncryptedBinaryField and adds getter/setter for handling encrypted data. |
| tom_eso/migrations/* | Contains migration files to alter and rename fields reflecting changes in encryption logic. |
| tom_eso/forms.py | Provides a custom ModelForm for ESOProfile that integrates the encryption logic. |
| tom_eso/eso_api.py | Adds a new TODO for potential renaming of a method for clarity. |
| tom_eso/eso.py | Updates reverse_lazy calls to use namespaced URL names with htmx attributes. |
| tom_eso/apps.py | Extends AppConfig with integration methods for profile details and field re-encryption. |
Comments suppressed due to low confidence (1)
tom_eso/models.py:104
- Typo in comment: '_chiphertext_p2_password' should be '_ciphertext_p2_password'.
# _chiphertext_p2_password is a memoryview object so it's probably Posgresql
The AppConfig.get_model() method takes a str with the name of the models.Model subclass, not the subclass is itself.
Here's was Copilot has to say: refactor ESOProfileForm and eso_profile_data to use session utility for encryption handling
This should use the verbose_name attribute of the model Field
Also, remove out-dated tests from before tom_common EncryptedProperty refactor.
jchate6
requested changes
Jul 17, 2025
Contributor
jchate6
left a comment
There was a problem hiding this comment.
This generally looks really good to me. Had a few comments.
| {% for field_data in profile_data_list %} | ||
| {% if field_data.value %} | ||
| <dt class="col-sm-6">{{ field_data.label }}</dt> | ||
| <dd class="col-sm-6">{{ field_data.value }}</dd> |
Contributor
There was a problem hiding this comment.
How do we feel about adding password here with a bunch of ******* instead of a value so people know it's editable from here?
Member
Author
There was a problem hiding this comment.
I've just made it so the password appears in the CharField so User can edit it to update it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I think these changes should be familiar to you @jchate6 because they are mostly about using the TOMToolkit integration points. Summary of the changes: