django-helpdesk / django-helpdesk

A Django application to manage tickets for an internal helpdesk. Formerly known as Jutda Helpdesk.
BSD 3-Clause "New" or "Revised" License
1.53k stars 636 forks source link

Hide default fields #503

Open willstott101 opened 7 years ago

willstott101 commented 7 years ago

Thanks for the awesome work here! It would be great to see progress on the public facing web interface as well as the email system. I may do some work on this in the near future.

I noticed that there is currently no easy way to hide default fields... for our use case we wanted to hide the due_date, priority, and assigned_to fields from non-staff users. I ended up achieving that with the following patch. However some view code relies on the existence of the fields in the fields dictionary, so this is not complete as a PR.

Luckily the field expected by the view was the assigned_to field which has it's own special snowflake setting: HELPDESK_CREATE_TICKET_HIDE_ASSIGNED_TO

diff --git a/helpdesk/forms.py b/helpdesk/forms.py
index 7ab2024..d1b6994 100644
--- a/helpdesk/forms.py
+++ b/helpdesk/forms.py
@@ -317,6 +317,8 @@ class TicketForm(AbstractTicketForm):
         """
         super(TicketForm, self).__init__(*args, **kwargs)
         self._add_form_custom_fields()
+        for f in getattr(settings, 'EA_HIDDEN_TICKET_FIELDS', []):
+            self.fields.pop(f, None)

     def save(self, user=None):
         """
@@ -369,6 +371,8 @@ class PublicTicketForm(AbstractTicketForm):
         """
         super(PublicTicketForm, self).__init__(*args, **kwargs)
         self._add_form_custom_fields(False)
+        for f in getattr(settings, 'EA_PUBLIC_HIDDEN_TICKET_FIELDS', []):
+            self.fields.pop(f, None)
gwasser commented 7 years ago

Thanks! I've been looking at the email system a bit myself but as far as I'm aware no one is working on the public interface yet, so greatly appreciate your help with patches!

Offhand I'm not sure I like the introduction of a new EA_HIDDEN_TICKET_FIELDS variable. Do you think we might be able to accomplish it by making a more basic form class, and then making the staff form a subclass with more features, rather than popping off stuff?

willstott101 commented 7 years ago

Yes that's indeed a possible technique. However that wouldn't really make it much more customisable. More recent versions on Django have a 'disabled' option for form fields. And there's also the HiddenField form Widget. I wonder if there's a way to implement this similar to how I've done it as a layer after building the class. But possibly at the template / widget level. Because:

  1. I personally believe customisation of fields is a very useful (and important) feature that we should strive to keep / enhance.

  2. View/Form code is expecting some of the fields to be present. Whilst yes we could change that code, I don't see why the view code shouldn't rely on the existence of form fields. Seems like a road that could lead to duplication of the form's blank field validation, might as well let the form do what it's there for.

  3. And this is where the method I introduced breaks down. Forms may be a good place to define default data different to the defaults on the model (again, forms are used for validation and pre-filling data). So removing them and their logic when all we actually want to do is hide them might not be the right thing to do.

Configuring

I agree that the setting isn't a pretty way to do it. But it fits what i was hoping to achieve and doesn't require knowledge of Form classes and Python's Class system to customise.

A more involved technique than the setting might be a django admin style, subclass and register pattern? Where the helpdesk implementation provides all functionality. And sublcasses can override the hidden / visible options of the form widgets, rather than the form fields? Those subclasses can then register themselves and have the helpdesk code look for them, but maybe that's over-complicating it.

Maybe including the default fields as instances in the database (along with the custom fields), and allowing hiding (but not deleting) via the admin could be the optimal solution for configuring?

Hiding the Fields

Simply not rendering fields in the form template would be my first thought of a nice implementation. However exactly what that looks like (in terms of how the template is provided the fields that SHOULD be rendered), I'm not sure. A naive implementation would also not prevent POSTing data manually.

Conclusion

I'll look at writing a form mixin which allows the hiding or disabling of fields by the Meta class and a disabled_fields attribute. Along with a cooperating template we can stop the fields from rendering and ignore received data. Whether the Meta.disabled_fields is optimal over the setting or database instances depends on what level of technical knowledge is intended to be known by an admin of this project.

arnuschky commented 6 years ago

I just added PR #583 that adds three new settings that allow the operator to pre-define the queue, priority and due_date fields for public tickets. If one of these settings are present the corresponding input field is hidden from the form.

It's simple but solves this problem for me in a relatively clean way.

gwasser commented 6 years ago

Thanks @arnuschky ! Would it be easy to add assigned_to as well to close out this ticket? We also need to update the documentation so users know about the new settings.

arnuschky commented 6 years ago

Should be fairly easy. The assigned_to setting didn't show up in my case, so I didn't add that. So basically we would set either a default owner or None, I assume?

Regarding the documentation, I can have a stab at documenting the settings.

arnuschky commented 6 years ago

Would it be easy to add assigned_to as well to close out this ticket?

So the field assigned_to is not defined for the public form, and therefore there is no need to hide it. What did you have in mind?

gwasser commented 6 years ago

@arnuschky oh thanks for following up! This ticket mentioned assigned_to but I suppose that's not on the public form. I'll leave it open to remind that we should change the logged in form too. Thanks for your work on the public form!

arnuschky commented 6 years ago

Ok. I added documentation of the new settings to the PR.

timthelion commented 8 months ago

You can already hide most fields (except the one for attachments with a combination of setting the _hide_fields_ and default values in the query parameters of the URL. We just need to find a better way of exposing this for non-iframe users.

https://github.com/django-helpdesk/django-helpdesk/blob/2b6ad7a2cffaf36d30a8414e6864a886bc135668/helpdesk/views/public.py#L102

See: https://github.com/django-helpdesk/django-helpdesk/blob/0.5.0/docs/integration.rst