Skip to content
This repository was archived by the owner on Sep 5, 2020. It is now read-only.

Comments

A number of customizations of django-helpdesk#1

Open
cworth-gh wants to merge 327 commits intoupstream-masterfrom
master
Open

A number of customizations of django-helpdesk#1
cworth-gh wants to merge 327 commits intoupstream-masterfrom
master

Conversation

@cworth-gh
Copy link

The first several of these (those with "merge commits") are likely suitable for upstream.

The last few commits are for things that just Nimbis wants, (like dropping the priority and due-date fields). We should perhaps figure out a way to do those outside of django-helpdesk (such as in overridden templates) or submit a more general fix upstream to allow us to configure the behavior we want. But for now, we can carry this fork.

This is in support of this card: https://trello.com/c/w37C53RJ/1185-tss-add-django-helpdesk-app

@cworth-gh
Copy link
Author

<fieldset>
<div class="form-group {% if field.errors %}has-error{% endif %}">
<label for='id_ticket'>{% trans "Ticket" %}</label>
<label for='id_ticket'>{% trans "Ticket number" %}</label>

Choose a reason for hiding this comment

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

Nitpick: s/number/Number/ ? I need to see this, so maybe it makes visual sense.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Fixed.

site = Site(domain='configure-django-sites.com')
return u"http://%s%s?ticket=%s&email=%s" % (
site.domain,
return self._absolute_uri(u"%s?ticket=%s&email=%s" % (

Choose a reason for hiding this comment

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

We're (you and I) are literally have a conversion right now about minimal changes, but if it should so interest you, you might make this cleaner with .format( (using args or kwargs) ). Dealers choice.

Copy link
Author

Choose a reason for hiding this comment

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

I actually considered that. But there are a bunch of cases of '%'-style formatting here.

What we'll probably want to do is to fix all of those in a standalone commit which we can send upstream.

Let's let our usage of this app. cook for a bit, and if we don't decide to throw it away, then we can do a bunch of cleanups targeting upstream.

@shaunbrady
Copy link

One or two small comments. Code looks good, especially considering some of the oddities of the upstream we discussed.

# If there is no email address in the query string, get it from
# the currently logged-in user
if email == '':
email = request.user.email
Copy link
Member

Choose a reason for hiding this comment

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

This has the effect of raising AttributeError: 'AnonymousUser' object has no attribute 'email' if a user attempts to view a ticket while not authenticated.

Copy link
Author

Choose a reason for hiding this comment

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

Yes. Better would be a redirect to a login page. I think I'd like to punt on this for now, (since we have two cards for doing the CMS integration and for fixing several similar issues related to permissions). I think one or both of those cards will end up addressing this.

@cworth-gh cworth-gh force-pushed the master branch 3 times, most recently from cc7e80f to 81090f0 Compare January 27, 2017 09:23
reduxionist and others added 21 commits April 6, 2017 07:59
`get_email` management command: Force message to unicode to avoid encoding issues
Would create a 500 when user omitted their email. Only a partial improve. Added a TODO: as this view still breaks if passing non-numeric characters to the ID.

I assume this needs a full overhaul really.
Never return None from views.public.create_ticket
…uture correspondence, set to view only initially, and update tests for CC
… ticket if necessary and not already the assignee, etc.
ericamador and others added 30 commits May 24, 2018 16:29
This is to make it easier to see what is being imported by this file.
This is dead code that is never used anywhere.
This is done for style purposes.
This commit fixes the line lengths in models.py to comply with flake8.
Previously, the code would first try to build a URL using the request object
if one was available. However, the "if self.request" was often raising
an uncaught AttributeError because the request didn't exist. We address
this problem by simply always using the current Site to build the URL.
This has the added benefit of reducing the cyclomatic complexity of this
function, making the behavior easier to reason about and maintain.
This will make the need to ensure that a Django Site object exists much
more obvious.
This will display the closed helpdesk tickets to staff users.
Previously users could only find these tickets if the ticket was
assigned to them.
"Owned by me" tickets will now filter based on all django-allauth
EmailAddress objects associated with the user the filed the ticket,
instead of just email address associated with the ticket.
All emails sent by helpdesk will no longer contain attachments. This is
done to prevent the leaking of potentially sensitive information.

Note that the ability to send attachments remains untouched but we are
just removing all the current uses of that feature.
Remove attachments from all sent emails
Todd changed the email he was using on his account. Since
django-helpdesk associates tickets to email addresses directly, he lost access to all of his old tickets. This management command has been written to solve any other cases like this, where a user switches from an old email to a new one.
Implement management command to update email addresses on tickets.
When tickets are updated or marked as 'Resolved', the email which
is sent to the user contains text instructing the users to reply
to the email if they need to provide additional information when
tickets are updated, or if they feel the resolution is not adequate.

In this commit, we remove that text from the email template, and
instead instruct users to re-open the ticket if the resolution
is not adequate, or view the ticket online to provide additional
information.
Remove reply to email text from email template
When new tickets are created, the courtesy email informing users
their tickets have been received previously contained a line
informing users to reply to the email using the ticket number in
the subject.

In this commit, that line is removed from the email. Instead, users
are given a link to the ticket on the site and asked to use that
to provide further information or view any updates.
This version of django-helpdesk removes text informing users
to reply to the email when users submit new tickets.

https://trello.com/c/ytuM4uIK
…il-text

Remove respond to email text when submitting new tickets.

https://trello.com/c/ytuM4uIK
The urlresolvers module no longer exists in Django 2.2.
This includes fixes necessary for Django 2.2 compatibility.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.