Skip to content

change: [M3-8583] - Add support ticket mocks to MSW 2.0 #10937

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

Merged

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Sep 13, 2024

Description 📝

This PR adds CRUD request handlers and seeding for support tickets to MSW 2.0.

Preview 📷

Screenshot 2024-10-04 at 2 54 55 PM

Mocked Data
Mock Request Result
getSupportTickets Screenshot 2024-09-13 at 2 59 34 PM
createSupportTicket and getSupportTicket Screenshot 2024-09-13 at 2 42 10 PM
createSupportTicketReply Screenshot 2024-09-13 at 2 48 27 PM
events from createSupportTicket and createSupportTicketReply Screenshot 2024-09-13 at 2 50 49 PM
closeSupportTicket Screenshot 2024-09-13 at 2 50 01 PM

How to test 🧪

Verification steps

(How to verify changes)

  • Check out this PR and use our dev tools to toggle the MSW on.
  • Change the "Base preset" to CRUD.
  • Go to http://localhost:3000/support/tickets?type=open and http://localhost:3000/support/tickets?type=closed and confirm that none of your real support tickets are visible.
  • Click Open New Ticket, fill out the support ticket modal, and submit.
  • Confirm that your mock ticket shows up as an open ticket.
  • Confirm that you can Add Update to your mock ticket.
  • Confirm that you can close your ticket, and the status updates accordingly.
  • Confirm that you can seed 100 support tickets using the MSW preset seeder.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@mjac0bs mjac0bs self-assigned this Sep 13, 2024
@github-actions github-actions bot added the Stale label Sep 29, 2024
@mjac0bs mjac0bs force-pushed the M3-8583-add-msw2-mocks-for-support-tickets branch from 485262a to 2fccb3e Compare October 4, 2024 21:10
@linode linode deleted a comment from github-actions bot Oct 4, 2024
@mjac0bs mjac0bs marked this pull request as ready for review October 4, 2024 21:56
@mjac0bs mjac0bs requested a review from a team as a code owner October 4, 2024 21:56
@mjac0bs mjac0bs requested review from dwiley-akamai and abailly-akamai and removed request for a team October 4, 2024 21:56
export const dbSeeders = [
linodesSeeder,
placementGroupSeeder,
supportTicketsSeeder,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want the ability to seed support tickets to test pagination, or should this really just be reserved for certain entities?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure! you've built it, it may come handy!

Comment on lines 32 to 38
// TODO: handle dynamic entity selection
entity: {
id: 1,
label: 'mock-linode-1',
type: 'linode',
url: '/v4/linode/instances/1',
},
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 wasn't sure how best to handle the entities associated with a support ticket. To mock the dropdown in the open support ticket dialog, we'd need to mock the get requests for all products. For the time being, it populates real account data.

I guess, when we create a support ticket, we could also mock create a linode so that the mock-linode-1 link would send us to the mock linode details page. It just won't match up with whatever entity the user selects when creating the ticket.

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 that's quite ok! it makes sense to have this populated by our actual products. We could mock that but that's extra work for little value

Copy link

github-actions bot commented Oct 4, 2024

Coverage Report:
Base Coverage: 86.95%
Current Coverage: 86.95%

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

thanks! looks good although something is missing

in dev-tools/load.ts we need this added to mergedContext

 supportReplies: [
   ...initialContext.supportReplies,
   ...(seedContext?.supportReplies || []),
],
supportTickets: [
  ...initialContext.supportTickets,
  ...(seedContext?.supportTickets || []),
],

Comment on lines 32 to 38
// TODO: handle dynamic entity selection
entity: {
id: 1,
label: 'mock-linode-1',
type: 'linode',
url: '/v4/linode/instances/1',
},
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 that's quite ok! it makes sense to have this populated by our actual products. We could mock that but that's extra work for little value

export const dbSeeders = [
linodesSeeder,
placementGroupSeeder,
supportTicketsSeeder,
Copy link
Contributor

Choose a reason for hiding this comment

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

sure! you've built it, it may come handy!

@mjac0bs
Copy link
Contributor Author

mjac0bs commented Oct 7, 2024

in dev-tools/load.ts we need this added to mergedContext

@abailly-akamai Thank you! I hadn't noticed this file. Made the change in e4915c1, but still trying to better understand what is happening here. When we enable dev tools, we want both the seeded data and the user-generated data to be part of context, but then we're passing all that context when setting up which handlers and preset to use (const baseHandlers = resolveMockPreset(mswPreset, mergedContext);), even though the CRUD preset is the only one that will use anything but initialContext? Basically just trying to understand how this set up works and how things were seemingly mostly working with the supportTickets and supportReplies missed in load.ts.

@mjac0bs mjac0bs requested a review from abailly-akamai October 7, 2024 17:26
@abailly-akamai
Copy link
Contributor

@mjac0bs if my memory serves me well, this is necessary to set things up. When i pulled your PR, the new properties were not populated in the store, so the mocking was not working. supportTickets and supportTickets were not part of the initial state. You probably did not noticed it since you may not have destroyed your indexedDB and once it was populated once it started working for you.

the logic in load.ts is a part of the code that needs improvement, I'll give you that :D
would be happy to see it get improved

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Great! thank you for adding this one

verified seeing ✅
verified crud operations ✅

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Oct 7, 2024
Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Newly-created mock ticket shows up as an open ticket ✅
Can Add Update to mock ticket ✅ (note about this below)
Can close ticket, and the status updates accordingly ✅
Can seed 100 support tickets using the MSW preset seeder ✅

When the ticket update is added, should it show as coming from test-account too as opposed to Customer Support?

Screen.Recording.2024-10-07.at.5.19.43.PM.mov

@mjac0bs
Copy link
Contributor Author

mjac0bs commented Oct 7, 2024

@dwiley-akamai Sure, that would be more accurate. I overwrote a few values for the support ticket reply factory in the createSupportTicketReply handler so now the reply doesn't look like it's coming from the wrong user. Thanks for catching.

Screenshot 2024-10-07 at 2 53 22 PM

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

🚀

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Oct 8, 2024
@mjac0bs mjac0bs merged commit 25ee272 into linode:develop Oct 8, 2024
20 checks passed
Copy link

cypress bot commented Oct 8, 2024

Cloud Manager E2E    Run #6639

Run Properties:  status check failed Failed #6639  •  git commit 25ee2727bd: change: [M3-8583] - Add support ticket mocks to MSW 2.0 (#10937)
Project Cloud Manager E2E
Run status status check failed Failed #6639
Run duration 28m 41s
Commit git commit 25ee2727bd: change: [M3-8583] - Add support ticket mocks to MSW 2.0 (#10937)
Committer Mariah Jacobs
View all properties for this run ↗︎

Test results
Tests that failed  Failures 1
Tests that were flaky  Flaky 3
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 428

Tests for review

Failed  cypress/e2e/core/linodes/clone-linode.spec.ts • 1 failed test

View Output Video

Test Artifacts
clone linode > can clone a Linode from Linode details page Screenshots Video
Flakiness  placementGroups/delete-placement-groups.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Placement Group deletion > can delete with Linodes assigned when unexpected error show up and retry Screenshots Video
Flakiness  parentChild/account-switching.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Parent/Child account switching > From Parent to Child > can switch from Parent account user to Proxy account user from Billing page Screenshots Video
Flakiness  volumes/create-volume.smoke.spec.ts • 1 flaky test

View Output Video

Test Artifacts
volumes > does not allow creation of a volume with invalid pricing from volumes landing Screenshots Video

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants