-
Notifications
You must be signed in to change notification settings - Fork 641
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
[online safety] Add safety categories to report abuse form #8635
Conversation
70453ba
to
74cc83f
Compare
74cc83f
to
cd41935
Compare
I think these are great changes, especially conditionally rendering the description that contains details that complainants should include. Once we replace the security/DMCA forms with associated portals, we should have far less support requests sent in with insufficient information. |
Should some of these be sub-topics i.e. another dropdown for specific selections? Seems a bit dense and overload in the single dropdown |
@@ -112,6 +145,42 @@ | |||
$('#report-abuse-form').show(); | |||
} | |||
|
|||
// We don't suggest the customer contact the owner in the case of safety violations | |||
if (val === 'ChildSexualExploitationOrAbuse' |
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.
If you move this logic to the Controller it can be unit tested.
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 not sure what we'd be testing. This is simply getting the value of the dropdown and turning messages, and in this case a control, on/off (and duplicates logic we're already using).
How about a grouping of options: |
@zhhyu That may be redundant as we have some other changes potentially coming down the pike, which may remove some of the pre-existing options (i.e. route customers to other locations for them and remove them from the dropdown). If this happens, most of what remains will be safety options. So, we could consider grouping options if these future changes determine that leaving the original ones untouched is better. For now I'd suggest leaving it as a simple list. This function (https://github.com/NuGet/NuGetGallery/pull/8635/files#diff-aad522d53d7c37d2d65abfa950169d203d7eff0393d17330cda261623c1f9ee7R46) doesn't support grouping (see
/cc @jcjiang |
I suggest to get another approval from @jcjiang , but I see that this change is behind the feature flag, so it's fine to check in because it will not be alive until we are ready. |
Addresses #8590
Spec: https://github.com/NuGet/Home/blob/dev/proposed/2021/NuGetOrgReportForm-SafetyChanges.md
[note that the gif below shows inconsistent "the" and "this" in dropdown. Since fixed.]