-
Notifications
You must be signed in to change notification settings - Fork 361
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
feat: [M3-7673] - Add Account Limit support ticket to Linode Create flow #10620
feat: [M3-7673] - Add Account Limit support ticket to Linode Create flow #10620
Conversation
c2fc27d
to
001ae14
Compare
a7482e9
to
272e246
Compare
…n Linode Create flow
Coverage Report: ❌ |
|
||
export interface TicketTypeData { | ||
dialogTitle: string; | ||
helperText: JSX.Element | string; | ||
ticketTitle?: string; |
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.
I updated TICKET_TYPE_MAP
to include a default ticket title for different ticket types, rather than handle this in the SupportLink. So, a SupportLink can either pass in a title as a prop (done with SMTP since it includes the linode name), or if they pass in a ticket type, we can get and prepopulate the right title for the ticket type via _prefilledTicketType
.
) : generalError && typeof generalError !== 'string' ? ( | ||
<Notice spacingTop={8} variant="error"> | ||
<SupportTicketError | ||
entityType="linode_id" |
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.
Here we are passing the entityType because it is used for the following fields:
The goal is to be able to reuse this account limit form for limits encountered on any entity type. Here, it's the Linode flow, and this is how to distinguish that. In a follow up PR, if this error were encountered when trying to create a Volume, we would show "volume" in place of Linode and not show the plan type field.
We don't have an entityId, so that remains undefined
, and we handle that this later before form submission here. Admittedly, it feels a little hacky. Open to other approaches, this just used our existing entityType
prop.
control={control} | ||
name="companyName" | ||
/> | ||
<SupportTicketProductSelectionFields ticketType="accountLimit" /> |
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.
Rather than have to use all the same queries for entities, I opted to modify SupportTicketProductSelectionFields
to include a field that will use those queries if the ticket type is accountLimit
.
{ticketType === 'accountLimit' ? ( | ||
<Controller | ||
render={({ field, fieldState }) => ( | ||
<TextField | ||
data-qa-ticket-number-of-entities | ||
errorText={fieldState.error?.message} | ||
helperText={`Current number of ${_entityType}: ${entityOptions.length}`} | ||
label={ACCOUNT_LIMIT_FIELD_NAME_TO_LABEL_MAP.numberOfEntities} | ||
name="numberOfEntities" | ||
onChange={field.onChange} | ||
placeholder={`Enter total number of ${_entityType}`} | ||
value={field.value} | ||
/> | ||
)} | ||
control={control} | ||
name="numberOfEntities" | ||
/> |
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.
render={({ field }) => ( | ||
<Autocomplete | ||
onChange={(e, id) => | ||
setValue('entityId', id ? String(id?.value) : '') | ||
} | ||
data-qa-ticket-entity-id | ||
disabled={entityOptions.length === 0} | ||
errorText={ | ||
entityError || | ||
fieldState.error?.message || | ||
errors.entityId?.message | ||
} | ||
inputValue={entityInputValue} | ||
label={ENTITY_ID_TO_NAME_MAP[entityType] ?? 'Entity Select'} | ||
loading={areEntitiesLoading} | ||
onInputChange={(e, value) => field.onChange(value ? value : '')} | ||
options={entityOptions} | ||
placeholder={`Select a ${ENTITY_ID_TO_NAME_MAP[entityType]}`} | ||
value={selectedEntity} | ||
onChange={(_e, type) => { | ||
// Don't reset things if the type hasn't changed. | ||
if (type.value === entityType) { | ||
return; | ||
} | ||
field.onChange(type.value); | ||
setValue('entityId', ''); | ||
setValue('entityInputValue', ''); | ||
clearErrors('entityId'); | ||
}} | ||
data-qa-ticket-entity-type | ||
disableClearable | ||
label="What is this regarding?" | ||
options={topicOptions} | ||
value={selectedTopic} | ||
/> | ||
)} | ||
control={control} | ||
name="entityInputValue" | ||
name="entityType" |
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.
The rest of this stuff didn't change, think it's just diff indentation.
numberOfEntities: 'Total number of entities you need?', | ||
useCase: | ||
'A detailed description of your use case and why you need access to more/larger entities', |
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.
The mocks use the entity typein place of "entities", but this was more straight-forward copy to keep generic instead of using string manipulation. Going to check with UX on whether "entity" is fine, or actual entity type is desired.
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.
This is the file that handles the support error, which has already been globally intercepted and the error.message has been converted to JSX. It is very similar to SupportError.tsx. I kept both for now, since this PR is only focused on the Linode Create flow, but in the future, I don't think we'll have much use for intercepting support errors globally, since what type of support error we will want to render depends on where we are in the app.
|
Do we expect the scroll-to-error to work for v1 or just v2? |
@jaalah-akamai It's supposed to be working for v1, based on our recent implementation... but you're right that it's not the case here. It's unrelated to this PR, but I'll make a bug ticket if there isn't an existing one so we can look into why |
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.
Form looks good!
Currently, I'm getting an API error that is not surfaced to the user
Details
Currently running into this error
{
"reason": "The provided ID did not match any existing Linodes.",
"field": "linode_id"
}
I see Cloud Manager sending this payload
{
"description": "**Business or company name**\nLinode\n\n**First and last name**\nBanks Nussman\n\n**Total number of entities you need?**\n637285\n\n**Which Linode plan do you need access to?**\nDedicated 5.3GB\n\n**A detailed description of your use case and why you need access to more/larger entities**\nI need to host my cat blog on 637285 Linodes\n\n**Links to public information - e.g. your business or application's website, Twitter profile, GitHub, etc.**\nhttps://test.com\n\n## testing\n\n- Testing markdown",
"linode_id": 0,
"summary": "Account Limit Increase"
}
// If this is an account limit ticket, we needed the entity type but won't actually send a valid entity selection. | ||
// Reset the entity type and id back to defaults. | ||
const _entityType = | ||
ticketType === 'accountLimit' ? 'general' : values.entityType; | ||
const _entityId = ticketType === 'accountLimit' ? '' : values.entityId; | ||
|
||
if (!['general', 'none'].includes(_entityType) && !_entityId) { | ||
form.setError('entityId', { | ||
message: `Please select a ${ENTITY_ID_TO_NAME_MAP[entityType]}.`, | ||
}); | ||
|
||
return; | ||
} | ||
return; | ||
} | ||
setSubmitting(true); | ||
|
||
createSupportTicket({ | ||
[_entityType]: Number(_entityId), |
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.
@bnussman-akamai Thanks for the catch - I thought I'd fixed this. I originally had this using resetField
and was intending to reset to form defaults. I also tried specifying the defaults (e.g. form.resetField('entityType', {defaultValue: 'general'})
) and wasn't seeing that work as I expected. Not sure if you know a correct way to do this? (Basically, we don't want to surface that error and we want to ignore the fact that we passed in an entity type, because its purpose was only to determine how we should populate the limit form fields; no actual Linode entity existed.)
With the above changes, we're not relying on updating form state, but this should be working:
Screen.Recording.2024-07-15.at.1.45.52.PM.mov
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.
I'm thinking resetField
wasn't working because once we're in the handleSubmit
, the values
won't change, but I might be missing something.
I think the updated solution looks good!
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.
Form works as expected ✅ Thanks for including testing!
Description 📝
This PR adds a new type of support ticket form: the account limit ticket. Currently, when creating a Linode and a user has:
or
a conditional error notice is displayed. Clicking the contact support link the user is redirected to the Support page and now, in this PR, a new support ticket dialog with custom account-limit-related fields is displayed. Once submitted by the user, the support team can determine if the user is eligible to have access to more Linodes and/or larger plans. According to Customer Support, this is one of the top requests they receive from users. Similar to the SMTP ticket form, this change aims to improve the user experience and reduce the amount of back-and-forth between customers and Support when handling requests.
Note
This PR only links this account limit support ticket in the Linode Create flow, to start. It will be added to the rest of the flows that users can encounter this notice in follow up PRs.
Follow up tasks:
Changes 🔄
accountLimit
Target release date 🗓️
7/22
Preview 📷
How to test 🧪
Prerequisites
(How to setup test environment)
Verification steps
(How to verify changes)
In admin, set your reputation to 0.
Fill out the linode form and try to create the linode.
Confirm that the UI displays an error notice when user has reached a limit on their account that prevents them from deploying the selected Linode plan:
A limit on your account is preventing the deployment of the selected Linode plan. To request access to the plan, please contact Support and provide the Linode plan name.
In admin, reset your reputation to its previous value and set your thing limit to 0.
Fill out the linode form and try to create the linode.
Confirm that the UI displays an error notice when user has reached a limit for the number of active services on their account:
You’ve reached a limit for the number of active services on your account. Please contact Support to request an increase and provide the total number of services you may need.
For each of the above possible limit errors:
Confirm that clicking "contact Support" redirects you to the support ticket landing page with a new Account Limit ticket open.
Confirm that the ticket title is always "Account Limit Increase" and the First/Last Name and Company fields are pre-populated, if you have that data on your account.
Try to submit the ticket and confirm validation errors show for any required fields that aren't filled out.
Fill out fields and submit the ticket. Confirm that you are redirected to the ticket details page, and the ticket body lists the questions and answers from the ticket.
Confirm no regressions to other ticket types
Remember to restore all values in admin to their state prior to testing!
Verify that all test cases in Cypress pass and the added test coverage for this PR looks good:
As an Author I have considered 🤔
Check all that apply