-
-
Notifications
You must be signed in to change notification settings - Fork 351
testplan-multi-clone #3985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
testplan-multi-clone #3985
Changes from 4 commits
66cdd4e
647e528
b20d701
8caa4c1
43a668e
c3d34cc
daf49de
b98c211
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,11 @@ | |
| dataTableJsonRPC('TestPlan.filter', params, callbackF, preProcessData) | ||
| }, | ||
| columns: [ | ||
| { | ||
| data: null, | ||
| orderable: false, | ||
| render: function () { return '<input type="checkbox" class="row-select">' } | ||
| }, | ||
| { | ||
| data: null, | ||
| defaultContent: '', | ||
|
|
@@ -165,7 +170,22 @@ | |
| processing: '<div class="spinner spinner-lg"></div>', | ||
| zeroRecords: 'No records found' | ||
| }, | ||
| order: [[1, 'asc']] | ||
| order: [[2, 'asc']] | ||
| }) | ||
|
|
||
| // header “select all” | ||
| $('#resultsTable thead').on('change', '#select-all', function () { | ||
| const checked = this.checked | ||
| $('#resultsTable tbody input.row-select') | ||
| .prop('checked', checked) | ||
| .trigger('change') | ||
| }) | ||
|
|
||
| // row checkbox handler | ||
| $('#resultsTable tbody').on('change', 'input.row-select', function () { | ||
| const $tr = $(this).closest('tr') | ||
| if (this.checked) table.row($tr).select() | ||
| else table.row($tr).deselect() | ||
| }) | ||
|
|
||
| // Add event listener for opening and closing nested test plans | ||
|
|
@@ -189,9 +209,44 @@ | |
| return false // so we don't actually send the form | ||
| }) | ||
|
|
||
| $('#btn_clone_bulk').click(function () { | ||
| const selectedTestPlans = getSelectedTestPlans() | ||
| if (selectedTestPlans.length === 0) { | ||
| alert('No test plans selected for cloning.') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This string must be translatable. See how it's done for other strings on dynamic pages ( |
||
| return false | ||
| } | ||
|
|
||
| window.location.assign(`/plan/clone/?p=${selectedTestPlans.join('&p=')}`) | ||
|
|
||
| }) | ||
|
|
||
| $('#id_product').change(updateVersionSelectFromProduct) | ||
| } | ||
|
|
||
| function getSelectedTestPlans () { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm, this feels like a duplicate. Need to check if we don't have something more generic which already does this. In testplans/static/testplans/js/get.js there is getSelectedTestCases() and IIRC there was another one in utils.js but let's leave this for later.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main differences between getSelectedTestPlans() and getSelectedTestCases() is that getplans has the handle selecting child nested elements where getcases does not do this. |
||
| const inputs = $('#resultsTable tbody input.row-select:checked').closest('tr') | ||
| const tpIds = [] | ||
|
|
||
| inputs.each(function (_, el) { | ||
| const id = $(el).closest('tr').find('td:nth-child(3)').text().trim() | ||
| if (id) { | ||
| tpIds.push(id) | ||
| } | ||
|
|
||
| // Check if the row has collapsed children and add their IDs | ||
| const row = $('#resultsTable').DataTable().row($(el).closest('tr')) | ||
| if (hiddenChildRows[id] && !row.child.isShown()) { | ||
| hiddenChildRows[id].forEach(function (childRow) { | ||
| const childId = $(childRow).find('td:nth-child(3)').text().trim() | ||
| if (childId) { | ||
| tpIds.push(childId) | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| return tpIds | ||
| } | ||
|
|
||
| function hideExpandedChildren (table, parentRow) { | ||
| const children = hiddenChildRows[parentRow.data().id] | ||
| children.forEach( | ||
|
|
@@ -214,5 +269,10 @@ | |
| // this is an array of previously hidden rows | ||
| const children = hiddenChildRows[data.id] | ||
| $(children).find('td').css('border', '0').css('padding-left', `${childPadding}px`) | ||
|
|
||
| if ($(parentRow).find('input.row-select').prop('checked')) { | ||
| $(children).find('input.row-select').prop('checked', true).trigger('change') | ||
| } | ||
|
|
||
| return $(children).show() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| {% extends "base.html" %} | ||
| {% load i18n %} | ||
| {% load static %} | ||
|
|
||
| {% block title %}{% trans "Clone TestPlan" %} - {{ test_plan.name }}{% endblock %} | ||
| {% block page_id %}page-testplans-mutable{% endblock %} | ||
|
|
||
| {% block contents %} | ||
| <div class="container-fluid container-cards-pf"> | ||
| <form class="form-horizontal" method="post" action="{% url "plans-multi-clone" %}"> | ||
| {% csrf_token %} | ||
|
|
||
| {% for plan in form.plan.field.queryset %} | ||
| <input type="hidden" name="plan" value="{{ plan.pk }}"> | ||
| {% endfor %} | ||
|
|
||
| <div class="form-group"> | ||
| <div class="col-md-1 col-lg-1"> | ||
| <label for="id_product">{% trans "Product" %}</label> | ||
| <a href="{% url 'admin:management_product_add' %}?_popup" id="add_id_product" alt="{% trans 'add new Product' %}" title="{% trans 'add new Product' %}">+</a> | ||
| </div> | ||
| <div class="col-md-3 col-lg-3 {% if form.product.errors %}has-error{% endif %}"> | ||
| <select id="id_product" name="product" class="form-control selectpicker"> | ||
| {% for product in form.product.field.queryset %} | ||
| <option value="{{ product.pk }}" {% if product.pk|escape == form.product.value|escape %}selected{% endif %}> | ||
| {{ product.name }} | ||
| </option> | ||
| {% endfor %} | ||
| </select> | ||
| {{ form.product.errors }} | ||
| </div> | ||
|
|
||
| <div class="col-md-1 col-lg-1"> | ||
| <label for="id_version">{% trans "Version" %}</label> | ||
| <a href="{% url 'admin:management_version_add' %}?_popup&product={{ form.product.value }}" | ||
| id="add_id_version" alt="{% trans 'add new Version' %}" title="{% trans 'add new Version' %}"> | ||
| + | ||
| </a> | ||
| </div> | ||
| <div class="col-md-3 col-lg-3 {% if form.version.errors %}has-error{% endif %}"> | ||
| <select id="id_version" name="version" class="form-control selectpicker"> | ||
| {% for product_version in form.version.field.queryset %} | ||
| <option value="{{ product_version.pk }}" {% if product_version.pk|escape == form.version.value|escape %}selected{% endif %}> | ||
| {{ product_version.value }} | ||
| </option> | ||
| {% endfor %} | ||
| </select> | ||
| {{ form.version.errors }} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="form-group"> | ||
| <label class="col-md-1 col-lg-1" for="id_copy_testcases">{% trans "Clone TCs" %}</label> | ||
| <div class="col-md-3 col-lg-3"> | ||
| <input class="bootstrap-switch" id="id_copy_testcases" name="copy_testcases" | ||
| type="checkbox" {% if form.copy_testcases.value %}checked{% endif %} data-on-text="{% trans 'Yes' %}" data-off-text="{% trans 'No' %}"> | ||
| <p class="help-block">{% trans "Clone or link existing TCs into new TP" %}</p> | ||
| </div> | ||
|
|
||
|
|
||
| <label class="col-md-1 col-lg-1" for="id_set_parent">{% trans "Parent TP" %}</label> | ||
| <div class="col-md-3 col-lg-3"> | ||
| <input class="bootstrap-switch" id="id_set_parent" name="set_parent" | ||
| type="checkbox" {% if form.set_parent.value %}checked{% endif %} data-on-text="{% trans 'Yes' %}" data-off-text="{% trans 'No' %}"> | ||
| <p class="help-block">{% trans "Set the source TP as parent of new TP" %}</p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="form-group"> | ||
| <div class="col-md-1 col-lg-1"> | ||
| <button type="submit" class="btn btn-default btn-lg">{% trans "Clone" %}</button> | ||
| </div> | ||
| </div> | ||
| </form> | ||
|
|
||
| </div><!-- /container --> | ||
| {% endblock %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| from django.contrib import messages | ||
| from django.contrib.auth.decorators import permission_required | ||
| from django.http import HttpResponsePermanentRedirect, HttpResponseRedirect | ||
| from django.shortcuts import render | ||
| from django.urls import reverse | ||
| from django.utils.decorators import method_decorator | ||
| from django.utils.translation import gettext_lazy as _ | ||
|
|
@@ -15,6 +17,7 @@ | |
| from tcms.management.models import Priority | ||
| from tcms.testcases.models import TestCaseStatus | ||
| from tcms.testplans.forms import ( | ||
| CloneMultiPlanForm, | ||
| ClonePlanForm, | ||
| NewPlanForm, | ||
| PlanNotifyFormSet, | ||
|
|
@@ -226,3 +229,64 @@ | |
| return HttpResponseRedirect( | ||
| reverse("test_plan_url_short", args=[cloned_plan.pk]) | ||
| ) | ||
|
|
||
|
|
||
| @method_decorator(permission_required("testplans.add_testplan"), name="dispatch") | ||
| class MultiClone(FormView): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not liking this one very much b/c it adds unnecessary code. IMO all could be done with the existing cloning code but needs more attention to the details. Let's sort the GUI part first and then will circle back to this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could remove the original single clone classes and code and change it to redirect to multiclone checking if there is only one and if so letting you change its name. |
||
| template_name = "testplans/multi_clone.html" | ||
| form_class = CloneMultiPlanForm | ||
|
|
||
| http_method_names = ["get", "post"] | ||
|
|
||
| def get(self, request): # pylint: disable=W0221 | ||
| if not self._is_request_data_valid(request, "p"): | ||
| return HttpResponseRedirect(request.META.get("HTTP_REFERER", "/")) | ||
Check warningCode scanning / CodeQL URL redirection from remote source Medium test
Untrusted URL redirection depends on a
user-provided value Error loading related location Loading |
||
|
|
||
| get_params = request.GET.copy() | ||
| get_params.setlist("plan", request.GET.getlist("p")) | ||
| del get_params["p"] | ||
|
|
||
| planids = get_params.getlist("plan") | ||
|
|
||
| clone_form = CloneMultiPlanForm(get_params) | ||
| clone_form.populate(plans=planids) | ||
|
|
||
| context = { | ||
| "form": clone_form, | ||
| } | ||
| return render(request, self.template_name, context) | ||
|
|
||
| def post(self, request): # pylint: disable=W0221 | ||
| if not self._is_request_data_valid(request): | ||
| return HttpResponseRedirect(request.META.get("HTTP_REFERER", "/")) | ||
Check warningCode scanning / CodeQL URL redirection from remote source Medium test
Untrusted URL redirection depends on a
user-provided value Error loading related location Loading |
||
|
|
||
| # Do the clone action | ||
| clone_form = CloneMultiPlanForm(request.POST) | ||
| clone_form.populate(plans=request.POST.getlist("plan")) | ||
|
|
||
| if clone_form.is_valid(): | ||
|
|
||
| for tp_src in clone_form.cleaned_data["plan"]: | ||
| tp_src.clone(name=tp_src.name, **clone_form.cleaned_data) | ||
|
|
||
| # Otherwise tell the user the clone action is successful | ||
| messages.add_message( | ||
| request, messages.SUCCESS, _("TestPlan cloning was successful") | ||
| ) | ||
| return HttpResponseRedirect(reverse("plans-search")) | ||
|
|
||
| # invalid form | ||
| messages.add_message(request, messages.ERROR, clone_form.errors) | ||
| return HttpResponseRedirect(request.META.get("HTTP_REFERER", "/")) | ||
Check warningCode scanning / CodeQL URL redirection from remote source Medium test
Untrusted URL redirection depends on a
user-provided value Error loading related location Loading |
||
|
|
||
| @staticmethod | ||
| def _is_request_data_valid(request, field_name="plan"): | ||
| request_data = getattr(request, request.method) | ||
|
|
||
| if field_name not in request_data: | ||
| messages.add_message( | ||
| request, messages.ERROR, _("At least one TestPlan is required") | ||
| ) | ||
| return False | ||
|
|
||
| return True | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not liking this in the header that much.
Can you move the "Select All" + the "Clone Selected" buttons into the button box above the table?
See how the rest of the buttons are defined. For icon, use fa-code-fork icon to match existing UI.
The order should be
[ ] <clone-selected> <excel> <.... the rest of the buttons>.Note: (a follow up improvement can be made to the rest of these table buttons to operate only on the current selection if present.