From 05c734926a367fe21fb5bc87432f13ac3da33b14 Mon Sep 17 00:00:00 2001 From: elias-boulharts Date: Mon, 11 Dec 2023 14:05:37 +0100 Subject: [PATCH] [Enhancement] "On hold" instead of "Need info" state --- CHANGELOG.md | 2 +- Squest/views.py | 8 +-- docs/administration/metrics.md | 6 +- docs/dev/request-state-machine.md | 19 ++--- docs/manual/administration/rbac.md | 27 +++++--- profiles/templatetags/squest_utils.py | 2 +- service_catalog/api/urls.py | 4 +- .../views/request_state_machine_api_view.py | 10 +-- .../commands/insert_testing_data.py | 2 +- ...st_options_alter_request_state_and_more.py | 28 ++++++++ service_catalog/models/request.py | 12 ++-- service_catalog/models/request_state.py | 2 +- service_catalog/urls.py | 2 +- service_catalog/views/color.py | 2 +- service_catalog/views/filters.py | 6 +- service_catalog/views/request.py | 10 +-- templates/home/home.html | 6 +- ...quest-need-info.html => request-hold.html} | 4 +- .../request_details/approval.html | 69 +++++++++++++------ .../test_request/test_request_patch.py | 4 +- .../test_api/test_request/test_request_put.py | 2 +- .../test_request_state_machine/test_accept.py | 2 +- .../test_request_state_machine/test_cancel.py | 2 +- .../{test_need_info.py => test_on_hold.py} | 32 ++++----- .../test_request_state_machine/test_reject.py | 4 +- .../test_urls/test_request/test_crud.py | 2 +- .../{test_need_info.py => test_on_hold.py} | 12 ++-- .../{test_need_info.py => test_on_hold.py} | 12 ++-- .../test_request/test_admin_request_view.py | 18 ++--- .../test_views/test_common/test_home.py | 2 +- 30 files changed, 188 insertions(+), 125 deletions(-) create mode 100644 service_catalog/migrations/0039_alter_request_options_alter_request_state_and_more.py rename templates/service_catalog/admin/request/{request-need-info.html => request-hold.html} (95%) rename tests/test_service_catalog/test_api/test_request/test_request_state_machine/{test_need_info.py => test_on_hold.py} (66%) rename tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/{test_need_info.py => test_on_hold.py} (71%) rename tests/test_service_catalog/test_urls/test_request/test_state_machine/{test_need_info.py => test_on_hold.py} (67%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 300a8e4b0..61082424f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Enhancement -- Add a transition in FSM to switch from "NEED INFO" to "ACCEPTED" +- Add a transition in FSM to switch from "ON HOLD" to "ACCEPTED" - Add field "enabled" in ApprovalWorkflow - Remove "operations" field in ApprovalWorkflow form when editing - Add the number of items displayed at the end of lists diff --git a/Squest/views.py b/Squest/views.py index 6a40e4aa4..70dd48491 100644 --- a/Squest/views.py +++ b/Squest/views.py @@ -45,8 +45,8 @@ def home(request): if request.user.has_perm('service_catalog.list_request'): context['total_request'] = sum([x["count"] for x in all_requests if x["state"] == RequestState.SUBMITTED]) - context['total_request_need_info'] = sum( - [x["count"] for x in all_requests if x["state"] == RequestState.NEED_INFO]) + context['total_request_on_hold'] = sum( + [x["count"] for x in all_requests if x["state"] == RequestState.ON_HOLD]) if request.user.has_perm('service_catalog.list_instance'): context['total_instance'] = sum([x["count"] for x in all_instances if x["state"] == InstanceState.AVAILABLE]) @@ -77,8 +77,8 @@ def home(request): x["state"] == RequestState.FAILED and x[ "instance__service"] == service.id]) - service_dict["need_info_requests"] = sum([x["count"] for x in all_requests if - x["state"] == RequestState.NEED_INFO and x[ + service_dict["hold_requests"] = sum([x["count"] for x in all_requests if + x["state"] == RequestState.ON_HOLD and x[ "instance__service"] == service.id]) service_dict["opened_supports"] = sum([x["count"] for x in all_supports if diff --git a/docs/administration/metrics.md b/docs/administration/metrics.md index d7e75ad4c..30cd6da1d 100644 --- a/docs/administration/metrics.md +++ b/docs/administration/metrics.md @@ -72,7 +72,7 @@ squest_request_per_state_total{state="ACCEPTED"} 4.0 squest_request_per_state_total{state="CANCELED"} 3.0 squest_request_per_state_total{state="COMPLETE"} 5.0 squest_request_per_state_total{state="FAILED"} 4.0 -squest_request_per_state_total{state="NEED_INFO"} 2.0 +squest_request_per_state_total{state="ON_HOLD"} 2.0 squest_request_per_state_total{state="PROCESSING"} 3.0 squest_request_per_state_total{state="REJECTED"} 5.0 squest_request_per_state_total{state="SUBMITTED"} 4.00 @@ -111,7 +111,7 @@ E.g: squest_request_total{service="VMWare",state="COMPLETE"} 3.0 squest_request_total{service="VMWare",state="PROCESSING"} 2.0 squest_request_total{service="VMWare",state="ACCEPTED"} 2.0 -squest_request_total{service="VMWare",state="NEED_INFO"} 1.0 +squest_request_total{service="VMWare",state="ON_HOLD"} 1.0 squest_request_total{service="VMWare",state="REJECTED"} 4.0 squest_request_total{service="VMWare",state="SUBMITTED"} 1.0 squest_request_total{service="VMWare",state="FAILED"} 1.0 @@ -125,7 +125,7 @@ squest_request_total{service="Kubernetes",state="SUBMITTED"} 1.0 squest_request_total{service="Kubernetes",state="COMPLETE"} 1.0 squest_request_total{service="Kubernetes",state="CANCELED"} 1.0 squest_request_total{service="Kubernetes",state="PROCESSING"} 1.0 -squest_request_total{service="Kubernetes",state="NEED_INFO"} 1.0 +squest_request_total{service="Kubernetes",state="ON_HOLD"} 1.0 ``` ### squest_support_total diff --git a/docs/dev/request-state-machine.md b/docs/dev/request-state-machine.md index 2fd767856..455acce91 100644 --- a/docs/dev/request-state-machine.md +++ b/docs/dev/request-state-machine.md @@ -12,29 +12,32 @@ graph TB style auto_accept fill:#80CBC4 instance_pending([instance pending]) submitted --> instance_pending + submitted --> |re-submit|submitted instance_pending --> auto_accept accepted[ACCEPTED] auto_accept -->|Yes| accepted admin_action_1{admin action} style admin_action_1 fill:#80DEEA auto_accept -->|No| admin_action_1 - need_info[NEED_INFO] - admin_action_1 -->|need_info| need_info + on_hold[ON_HOLD] + admin_action_1 -->|on_hold| on_hold + admin_action_1 -->|cancel| canceled + admin_action_1 -->|reject| rejected admin_action_1 -->|accept| accepted rejected[REJECTED] - need_info -->|reject| rejected - need_info -->|Submit| submitted + on_hold -->|reject| rejected canceled[CANCELED] - need_info --> |cancel|canceled + on_hold --> |cancel|canceled + on_hold --> |accept|accepted rejected --> |cancel|canceled - submitted --> |cancel|canceled - submitted -->|reject| rejected canceled --> |delete| deleted deleted((Deleted)) auto_pocess{auto process?} style auto_pocess fill:#80CBC4 accepted --> auto_pocess accepted -->|reject| rejected + accepted -->|review| accepted + accepted -->|cancel| canceled auto_pocess --> |Yes| operation_type admin_action_2{admin action} auto_pocess --> |No| admin_action_2 @@ -60,7 +63,7 @@ graph TB processing_ok --> |Yes| complete processing_ok --> |No| failed failed --> |retry| processing - failed --> |cancel| accepted + failed --> |review| accepted archived[ARCHIVED] complete -->|archive| archived archived -->|unarchive| complete diff --git a/docs/manual/administration/rbac.md b/docs/manual/administration/rbac.md index 76ca14699..20ee88d23 100644 --- a/docs/manual/administration/rbac.md +++ b/docs/manual/administration/rbac.md @@ -2,7 +2,8 @@ Role-based access control (RBAC), is a mechanism that restricts Squest access. It involves setting **permissions** to enable access to authorized users. Permissions are then grouped -into **Roles** and given to a scope which can be a _team_ or and _organizations_ or _global_. **RBAC** is the link between a role, a scope and a user. +into **Roles** and given to a scope which can be a _team_ or and _organizations_ or _global_. **RBAC** is the link +between a role, a scope and a user. The Squest RBAC system enable an administrator to grant users or groups the ability to perform an action on arbitrary subsets of objects in Squest. @@ -13,12 +14,13 @@ on arbitrary subsets of objects in Squest. Permission in Squest represent a relationship with following components: -- **Name:** A short description of the permission. +- **Name:** A short description of the permission. - **Codename:** A unique identifier for the permission with camel case format. - **Content type:** A Squest object (E.g: Request, Instance) For example, a permission named "Can request a day2 operation on instance" attached to the content type "instance". -This permission is required, like the name is suggesting, to create a request for a day 2 operation on an existing instance. +This permission is required, like the name is suggesting, to create a request for a day 2 operation on an existing +instance. All objects have **generic** CRUD (Create, Retrieve/List, Update, Delete) permissions by default: @@ -35,7 +37,7 @@ All objects have **generic** CRUD (Create, Retrieve/List, Update, Delete) permis **Specific** Squest permissions: | Short description | Codename | Object | -| ----------------------------------------------- | --------------------------- | ------------ | +|-------------------------------------------------|-----------------------------|--------------| | Can add users in global scope | add_users_globalscope | globalscope | | Can delete users in global scope | delete_users_globalscope | globalscope | | Can view users in global scope | view_users_globalscope | globalscope | @@ -57,7 +59,7 @@ All objects have **generic** CRUD (Create, Retrieve/List, Update, Delete) permis | Can accept request | accept_request | request | | Can archive request | archive_request | request | | Can cancel request | cancel_request | request | -| Can ask info request | need_info_request | request | +| Can hold request | hold_request | request | | Can process request | process_request | request | | Can reject request | reject_request | request | | Can re-submit request | re_submit_request | request | @@ -74,7 +76,8 @@ All objects have **generic** CRUD (Create, Retrieve/List, Update, Delete) permis ## Global permissions -Global permissions are permissions granted to all logged Squest user. Permissions are purely additive (there are no "deny" rules). +Global permissions are permissions granted to all logged Squest user. Permissions are purely additive (there are no " +deny" rules). !!!warning @@ -86,12 +89,14 @@ Owner permissions are permissions granted to the owner of an Instance, Request o Are considered as owner: - * `requester` for Instance - * `user` for Request - * `opened_by` for Support +* `requester` for Instance +* `user` for Request +* `opened_by` for Support -Configuring "view_instance" permissions within the Owner Permissions will grant users the ability to see all instances for which they are the requester. -Adding "view_support" permission will grant users the ability to see all supports related to their instances and supports they opened. +Configuring "view_instance" permissions within the Owner Permissions will grant users the ability to see all instances +for which they are the requester. +Adding "view_support" permission will grant users the ability to see all supports related to their instances and +supports they opened. !!!warning diff --git a/profiles/templatetags/squest_utils.py b/profiles/templatetags/squest_utils.py index c20c756bf..259d3687f 100644 --- a/profiles/templatetags/squest_utils.py +++ b/profiles/templatetags/squest_utils.py @@ -65,7 +65,7 @@ def generate_sidebar(user): 'permission_required': 'service_catalog.list_request', 'active': [ "request_list", "request_create", "request_edit", "request_delete", - "request_details", "request_cancel", "request_need_info", "request_re_submit", "request_reject", + "request_details", "request_cancel", "request_on_hold", "request_re_submit", "request_reject", "request_accept", "request_process", "request_archive", "request_unarchive", "request_approve", "request_bulk_delete", "request_archived_list", "requestmessage_edit", "requestmessage_create" ] diff --git a/service_catalog/api/urls.py b/service_catalog/api/urls.py index cb8853a6e..085c6eb51 100644 --- a/service_catalog/api/urls.py +++ b/service_catalog/api/urls.py @@ -24,8 +24,8 @@ name='api_request_archive'), path('request//cancel/', RequestStateMachine.as_view({'post': 'cancel'}), name='api_request_cancel'), - path('request//need-info/', RequestStateMachine.as_view({'post': 'need_info'}), - name='api_request_need_info'), + path('request//hold/', RequestStateMachine.as_view({'post': 'hold'}), + name='api_request_on_hold'), path('request//process/', RequestStateMachine.as_view({'post': 'process'}), name='api_request_process'), path('request//re-submit/', RequestStateMachine.as_view({'post': 're_submit'}), diff --git a/service_catalog/api/views/request_state_machine_api_view.py b/service_catalog/api/views/request_state_machine_api_view.py index 499dce680..a53d9efa3 100644 --- a/service_catalog/api/views/request_state_machine_api_view.py +++ b/service_catalog/api/views/request_state_machine_api_view.py @@ -100,14 +100,14 @@ def re_submit(self, request, pk=None): @swagger_auto_schema(request_body=MessageSerializer, responses={200: AdminRequestSerializer()}) @action(detail=True) - def need_info(self, request, pk=None): + def hold(self, request, pk=None): """ - Ask for more info : change the state of the request to 'NEED_INFO'. + Ask for more info : change the state of the request to 'ON_HOLD'. """ target_request = get_object_or_404(Request, id=pk) - if not request.user.has_perm('service_catalog.need_info_request', target_request): + if not request.user.has_perm('service_catalog.hold_request', target_request): raise PermissionDenied - if not can_proceed(target_request.need_info): + if not can_proceed(target_request.on_hold): raise PermissionDenied data = deepcopy(request.data) data['sender'] = request.user.id @@ -115,7 +115,7 @@ def need_info(self, request, pk=None): message = RequestMessageSerializer(data=data) if message.is_valid(): message.save() - target_request.need_info() + target_request.on_hold() target_request.save() send_mail_request_update(target_request, user_applied_state=request.user, message=message) return Response(AdminRequestSerializer(target_request).data, status=status.HTTP_200_OK) diff --git a/service_catalog/management/commands/insert_testing_data.py b/service_catalog/management/commands/insert_testing_data.py index 24676edf7..864bd9e32 100644 --- a/service_catalog/management/commands/insert_testing_data.py +++ b/service_catalog/management/commands/insert_testing_data.py @@ -65,7 +65,7 @@ def handle(self, *args, **options): type=OperationType.DELETE, service=service, job_template=job_templates.get(name="Demo Job Template")) - states = [RequestState.SUBMITTED, RequestState.FAILED, RequestState.ACCEPTED, RequestState.NEED_INFO, + states = [RequestState.SUBMITTED, RequestState.FAILED, RequestState.ACCEPTED, RequestState.ON_HOLD, RequestState.REJECTED, RequestState.CANCELED, RequestState.PROCESSING, RequestState.COMPLETE] for i in range(random.randint(1, 3)): for username in users: diff --git a/service_catalog/migrations/0039_alter_request_options_alter_request_state_and_more.py b/service_catalog/migrations/0039_alter_request_options_alter_request_state_and_more.py new file mode 100644 index 000000000..ac528b1d5 --- /dev/null +++ b/service_catalog/migrations/0039_alter_request_options_alter_request_state_and_more.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.6 on 2023-12-11 12:53 + +from django.db import migrations, models +import django_fsm + + +class Migration(migrations.Migration): + + dependencies = [ + ('service_catalog', '0038_alter_towersurveyfield_unique_together_and_more'), + ] + + operations = [ + migrations.AlterModelOptions( + name='request', + options={'default_permissions': ('add', 'change', 'delete', 'view', 'list'), 'ordering': ['-last_updated'], 'permissions': [('accept_request', 'Can accept request'), ('cancel_request', 'Can cancel request'), ('reject_request', 'Can reject request'), ('archive_request', 'Can archive request'), ('unarchive_request', 'Can unarchive request'), ('re_submit_request', 'Can re-submit request'), ('process_request', 'Can process request'), ('hold_request', 'Can hold request'), ('view_admin_survey', 'Can view admin survey'), ('list_approvers', 'Can view who can accept')]}, + ), + migrations.AlterField( + model_name='request', + name='state', + field=django_fsm.FSMIntegerField(choices=[(1, 'SUBMITTED'), (2, 'ON_HOLD'), (3, 'REJECTED'), (4, 'CANCELED'), (5, 'ACCEPTED'), (6, 'PROCESSING'), (7, 'COMPLETE'), (8, 'FAILED'), (9, 'ARCHIVED')], default=1), + ), + migrations.AlterField( + model_name='requesthook', + name='state', + field=models.IntegerField(choices=[(1, 'SUBMITTED'), (2, 'ON_HOLD'), (3, 'REJECTED'), (4, 'CANCELED'), (5, 'ACCEPTED'), (6, 'PROCESSING'), (7, 'COMPLETE'), (8, 'FAILED'), (9, 'ARCHIVED')]), + ), + ] diff --git a/service_catalog/models/request.py b/service_catalog/models/request.py index 4a148e8bb..5ff64c2fb 100644 --- a/service_catalog/models/request.py +++ b/service_catalog/models/request.py @@ -37,7 +37,7 @@ class Meta: ("unarchive_request", "Can unarchive request"), ("re_submit_request", "Can re-submit request"), ("process_request", "Can process request"), - ("need_info_request", "Can ask info request"), + ("hold_request", "Can hold request"), ("view_admin_survey", "Can view admin survey"), ("list_approvers", "Can view who can accept"), ] @@ -158,8 +158,8 @@ def can_process(self): def tower_job_url(self): return f"{self.operation.job_template.tower_server.url}/#/jobs/playbook/{self.tower_job_id}/output" - @transition(field=state, source=RequestState.SUBMITTED, target=RequestState.NEED_INFO) - def need_info(self): + @transition(field=state, source=RequestState.SUBMITTED, target=RequestState.ON_HOLD) + def on_hold(self): pass @transition(field=state, source=[RequestState.SUBMITTED], target=RequestState.SUBMITTED) @@ -168,7 +168,7 @@ def re_submit(self, save=True): if save: self.save() @transition(field=state, source=[RequestState.SUBMITTED, - RequestState.NEED_INFO, + RequestState.ON_HOLD, RequestState.REJECTED, RequestState.ACCEPTED], target=RequestState.CANCELED) def cancel(self): @@ -176,7 +176,7 @@ def cancel(self): self.instance.abort() self.instance.save() - @transition(field=state, source=[RequestState.SUBMITTED, RequestState.ACCEPTED, RequestState.NEED_INFO], + @transition(field=state, source=[RequestState.SUBMITTED, RequestState.ACCEPTED, RequestState.ON_HOLD], target=RequestState.REJECTED) def reject(self, user): if self.instance.state == InstanceState.PENDING: @@ -186,7 +186,7 @@ def reject(self, user): return True @transition(field=state, - source=[RequestState.ACCEPTED, RequestState.SUBMITTED, RequestState.FAILED, RequestState.NEED_INFO], + source=[RequestState.ACCEPTED, RequestState.SUBMITTED, RequestState.FAILED, RequestState.ON_HOLD], target=RequestState.ACCEPTED) def accept(self, user, save=True): self.accepted_by = user diff --git a/service_catalog/models/request_state.py b/service_catalog/models/request_state.py index c9fbcc0bd..93af504e2 100644 --- a/service_catalog/models/request_state.py +++ b/service_catalog/models/request_state.py @@ -3,7 +3,7 @@ class RequestState(IntegerChoices): SUBMITTED = 1, "SUBMITTED" - NEED_INFO = 2, "NEED_INFO" + ON_HOLD = 2, "ON_HOLD" REJECTED = 3, "REJECTED" CANCELED = 4, "CANCELED" ACCEPTED = 5, "ACCEPTED" diff --git a/service_catalog/urls.py b/service_catalog/urls.py index 9d161e0ff..64e8576f3 100644 --- a/service_catalog/urls.py +++ b/service_catalog/urls.py @@ -15,7 +15,7 @@ # Request State Machine path('request//cancel/', views.request_cancel, name='request_cancel'), - path('request//need-info/', views.request_need_info, name='request_need_info'), + path('request//hold/', views.request_hold, name='request_on_hold'), path('request//re-submit/', views.RequestReSubmitView.as_view(), name='request_re_submit'), path('request//reject/', views.request_reject, name='request_reject'), path('request//accept/', views.request_accept, name='request_accept'), diff --git a/service_catalog/views/color.py b/service_catalog/views/color.py index 4c6c06568..d6ffb80bd 100644 --- a/service_catalog/views/color.py +++ b/service_catalog/views/color.py @@ -4,7 +4,7 @@ map_dict_request_state = { RequestState.ACCEPTED: "primary", - RequestState.NEED_INFO: "warning", + RequestState.ON_HOLD: "warning", RequestState.SUBMITTED: "info", RequestState.REJECTED: "dark", RequestState.PROCESSING: "orange", diff --git a/service_catalog/views/filters.py b/service_catalog/views/filters.py index 7bbb183c3..a4d0671ee 100644 --- a/service_catalog/views/filters.py +++ b/service_catalog/views/filters.py @@ -60,8 +60,8 @@ def can_proceed_request_action(args): target_request = Request.objects.get(id=args.split(',')[1]) if target_action == "cancel": return can_proceed(target_request.cancel) - elif target_action == "need_info": - return can_proceed(target_request.need_info) + elif target_action == "on_hold": + return can_proceed(target_request.on_hold) elif target_action == "reject": return can_proceed(target_request.reject) elif target_action == "accept": @@ -154,7 +154,7 @@ def squest_date_format(date_to_format): @register.filter(name="map_color_next_state") def map_color_next_state(current_state, next_state): - if current_state == "NEED_INFO": + if current_state == "ON_HOLD": return "warning" if current_state == next_state: return "primary" diff --git a/service_catalog/views/request.py b/service_catalog/views/request.py index 3f620481c..45c8bf085 100644 --- a/service_catalog/views/request.py +++ b/service_catalog/views/request.py @@ -173,18 +173,18 @@ def requestmessage_edit(request, request_id, pk): @login_required -def request_need_info(request, pk): +def request_hold(request, pk): target_request = get_object_or_404(Request, id=pk) - if not request.user.has_perm('service_catalog.need_info_request', target_request): + if not request.user.has_perm('service_catalog.hold_request', target_request): raise PermissionDenied - if not can_proceed(target_request.need_info): + if not can_proceed(target_request.on_hold): raise PermissionDenied if request.method == "POST": form = RequestMessageForm(request.POST or None, request.FILES or None, sender=request.user, target_request=target_request) if form.is_valid(): message = form.save(send_notification=False) - target_request.need_info() + target_request.on_hold() target_request.save() send_mail_request_update(target_request, user_applied_state=request.user, message=message) return redirect(target_request.get_absolute_url()) @@ -199,7 +199,7 @@ def request_need_info(request, pk): 'target_request': target_request, 'breadcrumbs': breadcrumbs } - return render(request, "service_catalog/admin/request/request-need-info.html", context) + return render(request, "service_catalog/admin/request/request-hold.html", context) diff --git a/templates/home/home.html b/templates/home/home.html index 4212f9c12..f5d839130 100644 --- a/templates/home/home.html +++ b/templates/home/home.html @@ -35,7 +35,7 @@

Service overview

Instance Submitted Accepted - Need info + On hold Failed {% if can_list_support %} Supports @@ -80,9 +80,9 @@

Service overview

{% endif %} - {% if service.need_info_requests %} + {% if service.hold_requests %} {{ service.need_info_requests }} + class="btn btn-warning btn-lg">{{ service.hold_requests }} {% endif %} diff --git a/templates/service_catalog/admin/request/request-need-info.html b/templates/service_catalog/admin/request/request-hold.html similarity index 95% rename from templates/service_catalog/admin/request/request-need-info.html rename to templates/service_catalog/admin/request/request-hold.html index 066472937..c06daf8aa 100644 --- a/templates/service_catalog/admin/request/request-need-info.html +++ b/templates/service_catalog/admin/request/request-hold.html @@ -1,6 +1,6 @@ {% extends 'base.html' %} {% block title %} - #{{ target_request.id }} | Need info + #{{ target_request.id }} | On hold {% endblock %} {% block content %} @@ -31,7 +31,7 @@ {% endif %} {% endif %} -
{% csrf_token %} + {% csrf_token %} {% for field in form %}
diff --git a/templates/service_catalog/request_details/approval.html b/templates/service_catalog/request_details/approval.html index ce89305d5..221a784f9 100644 --- a/templates/service_catalog/request_details/approval.html +++ b/templates/service_catalog/request_details/approval.html @@ -1,10 +1,10 @@ {% has_perm request.user "service_catalog.cancel_request" object as can_cancel_request %} -{% has_perm request.user "service_catalog.need_info_request" object as can_need_info_request %} +{% has_perm request.user "service_catalog.hold_request" object as can_hold_request %} {% has_perm request.user "service_catalog.re_submit_request" object as can_re_submit_request %} {% has_perm request.user "service_catalog.reject_request" object as can_reject_request %} {% has_perm request.user "service_catalog.accept_request" object as can_accept_request %} {% has_perm request.user "service_catalog.process_request" object as can_process_request %} -{% has_perm request.user "service_catalog.can_list_approvers" object as can_list_approvers %} +{% has_perm request.user "service_catalog.list_approvers" object as can_list_approvers %} {% has_perm request.user "service_catalog.view_admin_spec_instance" object.instance as can_view_admin_spec_instance %} @@ -27,11 +27,15 @@

Requested by
- {% if can_view_admin_spec_instance %} - {% endif %} @@ -148,6 +152,7 @@

{{ step.approval_step.name }}

Admin review

{% if can_list_approvers %} + {% who_can_approve object as list_who_can_approve %}
Approvers @@ -164,7 +169,7 @@

Admin review

This request is waiting for an approval {% endif %} - {% if object.get_state_display == "NEED_INFO" %} + {% if object.get_state_display == "ON_HOLD" %}
This request is waiting for information as comment. @@ -223,12 +228,12 @@

Admin review

{% endif %} {% endwith %} - {% if object.get_state_display != "NEED_INFO" %} - {% with args_filter="need_info,"|addstr:object.id %} - {% if args_filter|can_proceed_request_action and can_need_info_request %} + {% if object.get_state_display != "ON_HOLD" %} + {% with args_filter="on_hold,"|addstr:object.id %} + {% if args_filter|can_proceed_request_action and can_hold_request %} + href="{% url 'service_catalog:request_on_hold' object.id %}"> Ask info {% endif %} @@ -279,25 +284,46 @@

Process the request

{{ object.processed_by }} {% endif %} - {% if can_process_request and object.get_state_display == "ACCEPTED" %} + {% if object.get_state_display == "ACCEPTED" %}
+ {% with args_filter="accept,"|addstr:object.id %} + {% if args_filter|can_proceed_request_action and can_accept_request %} + + Review + + {% endif %} + {% endwith %} + {% with args_filter="reject,"|addstr:object.id %} + {% if args_filter|can_proceed_request_action and can_reject_request %} + + Reject + + + {% endif %} + {% endwith %} + {% with args_filter="cancel,"|addstr:object.id %} + {% if args_filter|can_proceed_request_action and can_cancel_request %} + + Cancel + + + {% endif %} + {% endwith %} {% with args_filter="process,"|addstr:object.id %} {% if args_filter|can_proceed_request_action and can_process_request %} - {% with args_filter="accept,"|addstr:object.id %} - {% if args_filter|can_proceed_request_action and can_accept_request %} - - Review - - {% endif %} - {% endwith %} Process + {% endif %} {% endwith %}
@@ -315,7 +341,8 @@

Process the request

{% if object.date_complete %} - {{ object.date_complete | squest_date_format }} + {{ object.date_complete | squest_date_format }} {% endif %}

Processing

@@ -326,7 +353,7 @@

Processing

Request processed successfully. {% endif %} - {% if can_process_request and object.tower_job_id is not None%} + {% if can_process_request and object.tower_job_id is not None %} diff --git a/tests/test_service_catalog/test_api/test_request/test_request_patch.py b/tests/test_service_catalog/test_api/test_request/test_request_patch.py index a2318eabf..d79ee8c88 100644 --- a/tests/test_service_catalog/test_api/test_request/test_request_patch.py +++ b/tests/test_service_catalog/test_api/test_request/test_request_patch.py @@ -25,7 +25,7 @@ def setUp(self): ) self.patch_data = { 'operation': self.update_operation_test.id, - 'state': RequestState.NEED_INFO, + 'state': RequestState.ON_HOLD, } self.kwargs = { 'pk': self.test_request_standard_user_1.id @@ -35,7 +35,7 @@ def setUp(self): 'id': self.test_request_standard_user_1.id, 'fill_in_survey': self.test_request_standard_user_1.fill_in_survey, 'tower_job_id': self.test_request_standard_user_1.tower_job_id, - 'state': RequestState.NEED_INFO, + 'state': RequestState.ON_HOLD, 'operation': self.update_operation_test.id, 'user': UserSerializerNested(self.test_request_standard_user_1.user).data } diff --git a/tests/test_service_catalog/test_api/test_request/test_request_put.py b/tests/test_service_catalog/test_api/test_request/test_request_put.py index 30293cf6a..5cb700f7e 100644 --- a/tests/test_service_catalog/test_api/test_request/test_request_put.py +++ b/tests/test_service_catalog/test_api/test_request/test_request_put.py @@ -26,7 +26,7 @@ def setUp(self): self.put_data = { 'fill_in_survey': dict(), 'tower_job_id': 6, - 'state': RequestState.NEED_INFO, + 'state': RequestState.ON_HOLD, 'operation': self.update_operation_test.id, 'user': {'id': self.standard_user.id} } diff --git a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_accept.py b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_accept.py index 0584bd60a..88ff79ed0 100644 --- a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_accept.py +++ b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_accept.py @@ -3,7 +3,7 @@ from service_catalog.models import RequestState, Request, Instance from tests.test_service_catalog.base_test_request import BaseTestRequestAPI -AUTHORIZED_STATES = [RequestState.SUBMITTED, RequestState.ACCEPTED, RequestState.FAILED, RequestState.NEED_INFO] +AUTHORIZED_STATES = [RequestState.SUBMITTED, RequestState.ACCEPTED, RequestState.FAILED, RequestState.ON_HOLD] class TestApiRequestAccept(BaseTestRequestAPI): diff --git a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_cancel.py b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_cancel.py index e5b3d5568..e55955e1d 100644 --- a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_cancel.py +++ b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_cancel.py @@ -3,7 +3,7 @@ from service_catalog.models import RequestState, InstanceState from tests.test_service_catalog.base_test_request import BaseTestRequestAPI -AUTHORIZED_STATES = [RequestState.SUBMITTED, RequestState.NEED_INFO, RequestState.REJECTED, RequestState.ACCEPTED] +AUTHORIZED_STATES = [RequestState.SUBMITTED, RequestState.ON_HOLD, RequestState.REJECTED, RequestState.ACCEPTED] class TestApiRequestCancel(BaseTestRequestAPI): diff --git a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_need_info.py b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_on_hold.py similarity index 66% rename from tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_need_info.py rename to tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_on_hold.py index 4b443158a..65dc2d6d5 100644 --- a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_need_info.py +++ b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_on_hold.py @@ -6,19 +6,19 @@ AUTHORIZED_STATES = [RequestState.SUBMITTED] -class TestApiRequestNeedInfo(BaseTestRequestAPI): +class TestApiRequestOnHold(BaseTestRequestAPI): def setUp(self): - super(TestApiRequestNeedInfo, self).setUp() - self.need_info_url = reverse('api_request_need_info', kwargs={'pk': self.test_request.id}) + super(TestApiRequestOnHold, self).setUp() + self.on_hold_url = reverse('api_request_on_hold', kwargs={'pk': self.test_request.id}) - def _need_info(self, status=200, data=None): + def _on_hold(self, status=200, data=None): message_count = self.test_request.comments.count() - response = self.client.post(reverse('api_request_need_info', kwargs={'pk': self.test_request.id}), data=data) + response = self.client.post(reverse('api_request_on_hold', kwargs={'pk': self.test_request.id}), data=data) self.assertEqual(response.status_code, status) if status == 200: self.test_request.refresh_from_db() - self.assertEqual(self.test_request.state, RequestState.NEED_INFO) + self.assertEqual(self.test_request.state, RequestState.ON_HOLD) last_message = self.test_request.comments.order_by("-creation_date").first() if data: self.assertEqual(data["content"], last_message.content) @@ -27,38 +27,38 @@ def _need_info(self, status=200, data=None): else: self.assertEqual(self.test_request.comments.count(), message_count) - def test_admin_can_ask_need_info_request_with_comment(self): + def test_admin_can_ask_hold_request_with_comment(self): data = {"content": "test comment"} for state in AUTHORIZED_STATES: self.test_request.state = state self.test_request.save() - self._need_info(data=data) + self._on_hold(data=data) - def test_admin_can_ask_need_info_request_without_comment(self): + def test_admin_can_ask_hold_request_without_comment(self): for state in AUTHORIZED_STATES: self.test_request.state = state self.test_request.save() - self._need_info() + self._on_hold() - def test_admin_cannot_ask_need_info_request_in_forbidden_state(self): + def test_admin_cannot_ask_hold_request_in_forbidden_state(self): forbidden_state = RequestState.values for state in AUTHORIZED_STATES: forbidden_state.remove(state) for state in forbidden_state: self.test_request.state = state self.test_request.save() - self._need_info(status=403) + self._on_hold(status=403) - def test_user_cannot_ask_need_info_request(self): + def test_user_cannot_ask_hold_request(self): self.client.force_login(self.standard_user) for state in AUTHORIZED_STATES: self.test_request.state = state self.test_request.save() - self._need_info(status=403) + self._on_hold(status=403) - def test_cannot_ask_need_info_request_when_logout(self): + def test_cannot_ask_hold_request_when_logout(self): self.client.logout() for state in AUTHORIZED_STATES: self.test_request.state = state self.test_request.save() - self._need_info(status=403) + self._on_hold(status=403) diff --git a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_reject.py b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_reject.py index 48515c8df..790d81837 100644 --- a/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_reject.py +++ b/tests/test_service_catalog/test_api/test_request/test_request_state_machine/test_reject.py @@ -4,7 +4,7 @@ from tests.test_service_catalog.base_test_request import BaseTestRequestAPI -AUTHORIZED_STATES = [RequestState.SUBMITTED, RequestState.ACCEPTED, RequestState.NEED_INFO] +AUTHORIZED_STATES = [RequestState.SUBMITTED, RequestState.ACCEPTED, RequestState.ON_HOLD] class TestApiRequestReject(BaseTestRequestAPI): @@ -44,7 +44,7 @@ def test_admin_cannot_reject_request_in_forbidden_state(self): forbidden_state = RequestState.values forbidden_state.remove(RequestState.SUBMITTED) forbidden_state.remove(RequestState.ACCEPTED) - forbidden_state.remove(RequestState.NEED_INFO) + forbidden_state.remove(RequestState.ON_HOLD) for state in forbidden_state: self.test_request.state = state self.test_request.save() diff --git a/tests/test_service_catalog/test_api/test_urls/test_request/test_crud.py b/tests/test_service_catalog/test_api/test_urls/test_request/test_crud.py index 96a872639..e6e769cb7 100644 --- a/tests/test_service_catalog/test_api/test_urls/test_request/test_crud.py +++ b/tests/test_service_catalog/test_api/test_urls/test_request/test_crud.py @@ -28,7 +28,7 @@ def test_request_views(self): data={ 'fill_in_survey': dict(), 'tower_job_id': 6, - 'state': RequestState.NEED_INFO, + 'state': RequestState.ON_HOLD, 'operation': self.update_operation_test.id, 'user': {'id': self.standard_user.id} }, diff --git a/tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/test_need_info.py b/tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/test_on_hold.py similarity index 71% rename from tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/test_need_info.py rename to tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/test_on_hold.py index 509339a90..da189dc6a 100644 --- a/tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/test_need_info.py +++ b/tests/test_service_catalog/test_api/test_urls/test_request/test_state_machine/test_on_hold.py @@ -3,24 +3,24 @@ from tests.permission_endpoint import TestingGetContextView, TestingPostContextView, TestPermissionEndpoint -class TestServiceCatalogRequestNeedInfoPermissionsEndpoint(BaseTestRequestAPI, TestPermissionEndpoint): +class TestServiceCatalogRequestOnHoldPermissionsEndpoint(BaseTestRequestAPI, TestPermissionEndpoint): def setUp(self): super().setUp() self.test_request.state = RequestState.SUBMITTED self.test_request.save() - def test_need_info_view(self): + def test_on_hold_view(self): testing_view_list = [ TestingGetContextView( - url='api_request_need_info', - perm_str='service_catalog.need_info_request', + url='api_request_on_hold', + perm_str='service_catalog.hold_request', url_kwargs={'pk': self.test_request.id}, expected_status_code=405, expected_not_allowed_status_code=405 ), TestingPostContextView( - url='api_request_need_info', - perm_str='service_catalog.need_info_request', + url='api_request_on_hold', + perm_str='service_catalog.hold_request', url_kwargs={'pk': self.test_request.id}, data={ 'content': 'My comment' diff --git a/tests/test_service_catalog/test_urls/test_request/test_state_machine/test_need_info.py b/tests/test_service_catalog/test_urls/test_request/test_state_machine/test_on_hold.py similarity index 67% rename from tests/test_service_catalog/test_urls/test_request/test_state_machine/test_need_info.py rename to tests/test_service_catalog/test_urls/test_request/test_state_machine/test_on_hold.py index 2645d02bf..32df14d09 100644 --- a/tests/test_service_catalog/test_urls/test_request/test_state_machine/test_need_info.py +++ b/tests/test_service_catalog/test_urls/test_request/test_state_machine/test_on_hold.py @@ -3,22 +3,22 @@ from tests.permission_endpoint import TestingGetContextView, TestingPostContextView, TestPermissionEndpoint -class TestServiceCatalogRequestPermissionsNeedInfoView(BaseTestRequest, TestPermissionEndpoint): +class TestServiceCatalogRequestPermissionsOnHoldView(BaseTestRequest, TestPermissionEndpoint): def setUp(self): super().setUp() self.test_request.state = RequestState.SUBMITTED self.test_request.save() - def test_need_info_view(self): + def test_on_hold_view(self): testing_view_list = [ TestingGetContextView( - url='service_catalog:request_need_info', - perm_str='service_catalog.need_info_request', + url='service_catalog:request_on_hold', + perm_str='service_catalog.hold_request', url_kwargs={'pk': self.test_request.id}, ), TestingPostContextView( - url='service_catalog:request_need_info', - perm_str='service_catalog.need_info_request', + url='service_catalog:request_on_hold', + perm_str='service_catalog.hold_request', url_kwargs={'pk': self.test_request.id}, data={ 'content': 'My comment' diff --git a/tests/test_service_catalog/test_views/test_admin/test_request/test_admin_request_view.py b/tests/test_service_catalog/test_views/test_admin/test_request/test_admin_request_view.py index eb8af9710..238360cbc 100644 --- a/tests/test_service_catalog/test_views/test_admin/test_request/test_admin_request_view.py +++ b/tests/test_service_catalog/test_views/test_admin/test_request/test_admin_request_view.py @@ -35,11 +35,11 @@ def test_request_cancel(self): self.test_request.refresh_from_db() self.assertEqual(self.test_request.instance.state, InstanceState.ABORTED) - def test_request_need_info(self): + def test_request_on_hold(self): args = { 'pk': self.test_request.id } - url = reverse('service_catalog:request_need_info', kwargs=args) + url = reverse('service_catalog:request_on_hold', kwargs=args) data = { "content": "admin message" } @@ -48,19 +48,19 @@ def test_request_need_info(self): response = self.client.post(url, data=data) self.assertEqual(302, response.status_code) self.test_request.refresh_from_db() - self.assertEqual(self.test_request.state, RequestState.NEED_INFO) + self.assertEqual(self.test_request.state, RequestState.ON_HOLD) self.assertEqual(1, RequestMessage.objects.filter(request=self.test_request.id).count()) - def test_admin_cannot_request_need_info_on_forbidden_states(self): + def test_admin_cannot_request_on_hold_on_forbidden_states(self): args = { 'pk': self.test_request.id } - url = reverse('service_catalog:request_need_info', kwargs=args) + url = reverse('service_catalog:request_on_hold', kwargs=args) data = { "message": "admin message" } forbidden_states = [RequestState.CANCELED, RequestState.ACCEPTED, RequestState.PROCESSING, RequestState.FAILED, - RequestState.COMPLETE, RequestState.REJECTED, RequestState.ARCHIVED, RequestState.NEED_INFO] + RequestState.COMPLETE, RequestState.REJECTED, RequestState.ARCHIVED, RequestState.ON_HOLD] for forbidden_state in forbidden_states: self.test_request.state = forbidden_state self.test_request.save() @@ -86,7 +86,7 @@ def test_admin_cannot_request_re_submit_on_forbidden_states(self): 'pk': self.test_request.id } url = reverse('service_catalog:request_re_submit', kwargs=args) - forbidden_states = [RequestState.NEED_INFO, RequestState.REJECTED, RequestState.CANCELED, RequestState.ACCEPTED, + forbidden_states = [RequestState.ON_HOLD, RequestState.REJECTED, RequestState.CANCELED, RequestState.ACCEPTED, RequestState.PROCESSING, RequestState.COMPLETE, RequestState.FAILED, RequestState.ARCHIVED] for forbidden_state in forbidden_states: self.test_request.state = forbidden_state @@ -545,7 +545,7 @@ def test_admin_cannot_request_archive_on_forbidden_states(self): } url = reverse('service_catalog:request_archive', kwargs=args) forbidden_states = [RequestState.CANCELED, RequestState.ACCEPTED, RequestState.PROCESSING, RequestState.FAILED, - RequestState.REJECTED, RequestState.SUBMITTED, RequestState.NEED_INFO, + RequestState.REJECTED, RequestState.SUBMITTED, RequestState.ON_HOLD, RequestState.ARCHIVED] for forbidden_state in forbidden_states: self.test_request.state = forbidden_state @@ -559,7 +559,7 @@ def test_admin_cannot_request_unarchive_on_forbidden_states(self): } url = reverse('service_catalog:request_unarchive', kwargs=args) forbidden_states = [RequestState.CANCELED, RequestState.ACCEPTED, RequestState.PROCESSING, RequestState.FAILED, - RequestState.REJECTED, RequestState.SUBMITTED, RequestState.NEED_INFO, + RequestState.REJECTED, RequestState.SUBMITTED, RequestState.ON_HOLD, RequestState.COMPLETE] for forbidden_state in forbidden_states: self.test_request.state = forbidden_state diff --git a/tests/test_service_catalog/test_views/test_common/test_home.py b/tests/test_service_catalog/test_views/test_common/test_home.py index 51fd744d0..fccf584c5 100644 --- a/tests/test_service_catalog/test_views/test_common/test_home.py +++ b/tests/test_service_catalog/test_views/test_common/test_home.py @@ -19,7 +19,7 @@ def test_admin_get_home(self): def test_customer_get_home(self): self.client.login(username=self.standard_user.username, password=self.common_password) not_expected_context_list = ['total_user_without_quota_scopes', 'total_user'] - expected_context_list = ['total_request', 'total_instance', 'total_request_need_info', 'total_support_opened'] + expected_context_list = ['total_request', 'total_instance', 'total_request_on_hold', 'total_support_opened'] response = self.client.get(self.url) self.assertEqual(200, response.status_code) for expected_context in expected_context_list: