Skip to content
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

Enhancement/case escalation modal #2541

Merged
merged 27 commits into from Sep 28, 2022
Merged

Conversation

kevgliss
Copy link
Contributor

@kevgliss kevgliss commented Sep 20, 2022

Adds the ability to modify incident details before a case is escalated via a dedicated dialog.

Screen Shot 2022-09-20 at 1 21 56 PM
Screen Shot 2022-09-26 at 1 52 57 PM
Screen Shot 2022-09-20 at 4 21 45 PM
Screen Shot 2022-09-20 at 4 22 03 PM

@kevgliss kevgliss added the enhancement New feature or request label Sep 20, 2022
@lgtm-com
Copy link

lgtm-com bot commented Sep 21, 2022

This pull request introduces 1 alert when merging 81b76c8 into 06a294c - view on LGTM.com

new alerts:

  • 1 for Syntax error

@mvilanova
Copy link
Contributor

I'll run the code locally and circle back with feedback.

@lgtm-com
Copy link

lgtm-com bot commented Sep 21, 2022

This pull request introduces 1 alert when merging d4c9a5e into 0ec5fef - view on LGTM.com

new alerts:

  • 1 for Syntax error

@kevgliss kevgliss marked this pull request as draft September 26, 2022 17:03
@kevgliss kevgliss assigned sfault and unassigned sfault Sep 26, 2022
@kevgliss kevgliss changed the title [WIP] Enhancement/case escalation modal Enhancement/case escalation modal Sep 26, 2022
@kevgliss kevgliss marked this pull request as ready for review September 26, 2022 21:38
@lgtm-com
Copy link

lgtm-com bot commented Sep 26, 2022

This pull request introduces 1 alert when merging 9d542c6 into c05ef08 - view on LGTM.com

new alerts:

  • 1 for Unused import

src/dispatch/case/flows.py Outdated Show resolved Hide resolved
src/dispatch/case/views.py Outdated Show resolved Hide resolved
src/dispatch/case/views.py Outdated Show resolved Hide resolved
src/dispatch/case/views.py Outdated Show resolved Hide resolved
src/dispatch/case/flows.py Outdated Show resolved Hide resolved
src/dispatch/static/dispatch/src/case/store.js Outdated Show resolved Hide resolved
src/dispatch/static/dispatch/src/case/store.js Outdated Show resolved Hide resolved
Comment on lines 111 to 114
pluginInstance.plugin.type | capitalize
}}</v-list-item-title>
<v-list-item-subtitle>{{
pluginInstance.plugin.description
Copy link
Contributor

Choose a reason for hiding this comment

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

I think showing the plugin type name and description instead of the resource name and description like before might be confusing for users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is technically new functionality and won't affect the current submissions, but I agree they should be the same, afaik before we had one off strings that weren't derived from anywhere. This was my attempt as generalizing them. We can either use these strings, add new ones or take the hit and make it so each reasource is rendered individually.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be consistent across the app and showing plugin info instead of resource info is confusing. When I look at the screenshot and I see the plugin title and description under incident resources it makes me think I created a plugin not a resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand, but we have to choose how we accomplish this, our options are 1) Use these strings 2) Create new strings 3) Render each resource individually. It sounds like you prefer option 3?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I prefer # 3 as it's more clear for the user and consistent with what we already have.

kevgliss and others added 2 commits September 27, 2022 10:04
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
…es.vue

Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 27, 2022

This pull request introduces 1 alert when merging 188e8ca into c05ef08 - view on LGTM.com

new alerts:

  • 1 for Unused import

kevgliss and others added 2 commits September 27, 2022 14:25
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
kevgliss and others added 6 commits September 27, 2022 14:39
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
Co-authored-by: Marc Vilanova <39573146+mvilanova@users.noreply.github.com>
@mvilanova mvilanova merged commit 352a909 into master Sep 28, 2022
@mvilanova mvilanova deleted the enhancement/case-escalation-modal branch September 28, 2022 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants