[PATCH v4 5/5] patch-list: add inline dropdown for delegate and state one-off changes
Daniel Axtens
dja at axtens.net
Fri Aug 27 00:29:01 AEST 2021
Ah, that is but one of the issues - I forgot to check what happened if
you were logged in as a normal user rather than a maintainer. Add this too:
diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index d41c4609fe6e..f1acfb3a599e 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -280,7 +280,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
# but we will need to follow the state and submitter relations for
# rendering the list template
patches = patches.select_related('state', 'submitter', 'delegate',
- 'series')
+ 'series', 'submitter__user')
patches = patches.only('state', 'submitter', 'delegate', 'project',
'series__name', 'name', 'date', 'msgid')
Daniel Axtens <dja at axtens.net> writes:
>> TODO: The loading of the patch-list page is very slow now because it
>> seems that for each call to check if a patch is editable by a user, the
>> db is accessed. Changes in the backend need to be made so this is done
>> with only done with only one call to the db.
>
> AFAICT, the issue is that the code does a new db query for every
> patch.is_editable() It didn't make things noticable slow for me, but I
> do see an explosion in query volume. Anyway, try the following:
>
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 58e4c51e9716..c29f9a988fd5 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -93,10 +93,14 @@ class Project(models.Model):
> send_notifications = models.BooleanField(default=False)
> use_tags = models.BooleanField(default=True)
>
> + @cached_property
> + def maintainer_users(self):
> + return [x.user for x in self.maintainer_project.all().only('user')]
> +
> def is_editable(self, user):
> if not user.is_authenticated:
> return False
> - return self in user.profile.maintainer_projects.all()
> + return user in self.maintainer_users
>
> @cached_property
> def tags(self):
>
>
> FWIW, I still think more than a few maintainers would be surprised that
> delegates are editable by normal users... I still think we probably want
> to make some guardrails available for projects. Not entirely sure what
> that should look like just yet but that's where my head is at.
>
> Kind regards,
> Daniel
>
>>
>> Signed-off-by: Raxel Gutierrez <raxel at google.com>
>> ---
>> htdocs/README.rst | 6 +++
>> htdocs/js/patch-list.js | 52 ++++++++++++++++++-
>> .../patchwork/partials/patch-list.html | 35 +++++++++++--
>> patchwork/views/__init__.py | 6 +++
>> 4 files changed, 94 insertions(+), 5 deletions(-)
>>
>> diff --git a/htdocs/README.rst b/htdocs/README.rst
>> index 6c435124..32550119 100644
>> --- a/htdocs/README.rst
>> +++ b/htdocs/README.rst
>> @@ -133,6 +133,12 @@ js
>>
>> Part of Patchwork.
>>
>> +``patch-list.js.``
>> + Event helpers and other application logic for patch-list.html. These
>> + support patch list manipulation (e.g. inline dropdown changes).
>> +
>> + Part of Patchwork.
>> +
>> ``selectize.min.js``
>> Selectize is the hybrid of a ``textbox`` and ``<select>`` box. It's jQuery
>> based and it has autocomplete and native-feeling keyboard navigation; useful
>> diff --git a/htdocs/js/patch-list.js b/htdocs/js/patch-list.js
>> index 6ae13721..526f5370 100644
>> --- a/htdocs/js/patch-list.js
>> +++ b/htdocs/js/patch-list.js
>> @@ -1,5 +1,32 @@
>> +import { updateProperty } from "./rest.js";
>> +
>> $( document ).ready(function() {
>> - $("#patch-list").stickyTableHeaders();
>> + let inlinePropertyDropdowns = $("td > select[class^='change-property-']");
>> + $(inlinePropertyDropdowns).each(function() {
>> + // Store previous dropdown selection
>> + $(this).data("prevProperty", $(this).val());
>> + });
>> +
>> + // Change listener for dropdowns that change an individual patch's delegate and state properties
>> + $(inlinePropertyDropdowns).change((event) => {
>> + const property = event.target.getAttribute("value");
>> + const { url, data } = getPatchProperties(event.target, property);
>> + const updateMessage = {
>> + 'error': "No patches updated",
>> + 'success': "1 patch updated",
>> + };
>> + updateProperty(url, data, updateMessage).then(isSuccess => {
>> + if (!isSuccess) {
>> + // Revert to previous selection
>> + $(event.target).val($(event.target).data("prevProperty"));
>> + } else {
>> + // Update to new previous selection
>> + $(event.target).data("prevProperty", $(event.target).val());
>> + }
>> + });
>> + });
>> +
>> + $("#patchlist").stickyTableHeaders();
>>
>> $("#check-all").change(function(e) {
>> if(this.checked) {
>> @@ -9,4 +36,25 @@ $( document ).ready(function() {
>> }
>> e.preventDefault();
>> });
>> -});
>> \ No newline at end of file
>> +
>> + /**
>> + * Returns the data to make property changes to a patch through PATCH request.
>> + * @param {Element} propertySelect Property select element modified.
>> + * @param {string} property Patch property modified (e.g. "state", "delegate")
>> + * @return {{property: string, value: string}}
>> + * property: Property field to be modified in request.
>> + * value: New value for property to be modified to in request.
>> + */
>> + function getPatchProperties(propertySelect, property) {
>> + const selectedOption = propertySelect.options[propertySelect.selectedIndex];
>> + const patchId = propertySelect.parentElement.parentElement.dataset.patchId;
>> + const propertyValue = (property === "state") ? selectedOption.text
>> + : (selectedOption.value === "*") ? null : selectedOption.value
>> + const data = {};
>> + data[property] = propertyValue;
>> + return {
>> + "url": `/api/patches/${patchId}/`,
>> + "data": data,
>> + };
>> + }
>> +});
>> diff --git a/patchwork/templates/patchwork/partials/patch-list.html b/patchwork/templates/patchwork/partials/patch-list.html
>> index aeb26aa8..7d2d2cc9 100644
>> --- a/patchwork/templates/patchwork/partials/patch-list.html
>> +++ b/patchwork/templates/patchwork/partials/patch-list.html
>> @@ -5,7 +5,7 @@
>> {% load static %}
>>
>> {% block headers %}
>> - <script src="{% static "js/patch-list.js" %}"></script>
>> + <script type="module" src="{% static "js/patch-list.js" %}"></script>
>> {% endblock %}
>>
>> {% include "patchwork/partials/filters.html" %}
>> @@ -187,8 +187,37 @@
>> <td id="patch-checks:{{patch.id}}" class="text-nowrap">{{ patch|patch_checks }}</td>
>> <td id="patch-date:{{patch.id}}" class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td>
>> <td id="patch-submitter:{{patch.id}}">{{ patch.submitter|personify:project }}</td>
>> - <td id="patch-delegate:{{patch.id}}">{{ patch.delegate.username }}</td>
>> - <td id="patch-state:{{patch.id}}">{{ patch.state }}</td>
>> + <td id="patch-delegate:{{patch.id}}">
>> + {% if patch.is_editable %}
>> + <select class="change-property-delegate" value="delegate">
>> + {% if not patch.delegate.username %}
>> + <option value="*" selected>No delegate</option>
>> + {% else %}
>> + <option value="*" selected>{{ patch.delegate.username }}</option>
>> + {% endif %}
>> + {% for maintainer in maintainers %}
>> + <option value="{{ maintainer.id }}">{{ maintainer.name }}</option>
>> + {% endfor %}
>> + </select>
>> + {% else %}
>> + {{ patch.delegate.username }}
>> + {% endif %}
>> + </td>
>> + <td id="patch-state:{{patch.id}}">
>> + {% if patch.is_editable %}
>> + <select class="change-property-state" value="state">
>> + {% for state in states %}
>> + {% if state.name == patch.state.name %}
>> + <option value="{{ patch.state.ordering }}" selected>{{ patch.state }}</option>
>> + {% else %}
>> + <option value="{{ state.ordering }}">{{ state.name }}</option>
>> + {% endif %}
>> + {% endfor %}
>> + </select>
>> + {% else %}
>> + {{ patch.state }}
>> + {% endif %}
>> + </td>
>> </tr>
>> {% empty %}
>> <tr>
>> diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
>> index 5da8046d..d41c4609 100644
>> --- a/patchwork/views/__init__.py
>> +++ b/patchwork/views/__init__.py
>> @@ -16,6 +16,7 @@ from patchwork.models import Bundle
>> from patchwork.models import BundlePatch
>> from patchwork.models import Patch
>> from patchwork.models import Project
>> +from patchwork.models import State
>> from patchwork.models import Check
>> from patchwork.paginator import Paginator
>>
>> @@ -177,6 +178,8 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
>> 'project': project,
>> 'projects': Project.objects.all(),
>> 'filters': filters,
>> + 'maintainers': project.maintainer_project.all(),
>> + 'states': State.objects.all(),
>> }
>>
>> # pagination
>> @@ -287,6 +290,9 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
>> Prefetch('check_set', queryset=Check.objects.only(
>> 'context', 'user_id', 'patch_id', 'state', 'date')))
>>
>> + for patch in patches:
>> + patch.is_editable = patch.is_editable(user)
>> +
>> paginator = Paginator(request, patches)
>>
>> context.update({
>> --
>> 2.33.0.rc2.250.ged5fa647cd-goog
>>
>> _______________________________________________
>> Patchwork mailing list
>> Patchwork at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list