This repository was archived by the owner on Sep 5, 2020. It is now read-only.
Update submitter email on email update#24
Open
kevinrobbins wants to merge 9 commits into
Open
Conversation
We need to be able to override the ready() method on the AppConfig in order to register our signals module with Django.
When a user changed their primary email address, they would lose access to their tickets because they were associated with the old email address. This signal updates their tickets to the new email address so that the users will still be able to access their tickets.
Because this is a pre_save signal, if a user is about to be created, there is no user to query for and an exception is thrown. Here we change the get() to a filter() and check to see if a user was returned. If no user is found, then we can just return because they would not have any tickets to update.
The name update_ticket_submitter_email more accurately describes what the signal receiver does.
We can't use 0.2.6+nimbis.23 because that version has already been created in preparation for the django upgrade. We want to release the site under django 1.11 which is not compatible with 0.2.6+nimbis.23.
Author
|
When merging, just take the version in setup.py in master. This version is just so we can release this pre-django upgrade. |
Author
|
I'm not worried about the checks. They are failing because of flake8 errors and I don't want to go in and fix dozens of errors in a package we mostly don't maintain. |
The following ISE was being thrown when users submitted a follow-up response on a ticket and attaching any file: Exception Type: FileNotFoundError at /support/tickets/<TICKET_ID>/update/ Exception Value: [Errno 2] No such file or directory: 'helpdesk/attachments/<SITE_ID-TICKET_ID>/...' In this commit we switched to using S3 storage for attachments in send_templated_mail: 7807b12 However, we did not apply that same change in the 'if six.PY3' consequent. This path was not hit when were were running sites using python2. Once we switched to python3, the bug fixed in the commit above re-appeared. Additionally, the latest version of django-helpdesk does not include this if-then statement. References: django-helpdesk#721 django-helpdesk@b43e608#diff-798c61915598ee76b4cb2ee8be71c807
Fix bug with attaching files when submitting follow-ups on tickets.
abannerjee
reviewed
Aug 21, 2020
|
@kevinrobbins Reviewed the code and left one comment which I think should be addressed/looked-at before merging. I also recommend just rebasing this against master and updating |
We don't need to update the ticket if the email address didn't change so we can exit early. Here we also condense the early exit logic so that it all happens in a single, unbroken chunk.
This will limit potential race conditions by only updating the submitter_email field. This way if some other property on the ticket is updated (for example by an admin) while the email address is being updated, we don't lose any other data.
Author
|
@abannerjee I made the changes you suggested. Once you re-review, rebase and create a version 24. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
This PR solves the issue where users change their email address and lose access to their tickets. This was because tickets are associated to users by the email address tied to the User object. This PR addresses this by creating a signal in helpdesk that detects changes to the User object and updates the email address appropriately.