-
Notifications
You must be signed in to change notification settings - Fork 83
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
OCC-169: Add user editor for the non-address checkout fields too #314
Conversation
src/Modules/OrchardCore.Commerce/Extensions/UserManagerExtensions.cs
Outdated
Show resolved
Hide resolved
else | ||
{ | ||
var errors = _updateModelAccessor.ModelUpdater.GetModelErrorMessages(); | ||
foreach (var error in errors.WhereNot(string.IsNullOrEmpty)) await _notifier.ErrorAsync(H[error]); |
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 pretty sure the validation errors are already localized so this should rather be H["{0}", error]
.
/// if the corresponding <see cref="User"/> is not <see langword="null"/>, returns <see langword="null"/> otherwise. | ||
/// </summary> | ||
public static async Task<UserDetailsPart> GetUserDetailsAsync(this UserManager<IUser> userManager, ClaimsPrincipal principal) => | ||
await userManager.GetUserAsync(principal) is User user ? user.As<ContentItem>(UserDetails)?.As<UserDetailsPart>() : null; |
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 write (await userManager.GetUserAsync(principal) as User)?
instead of user
then the conditional expression can be avoided.
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.
(await userManager.GetUserAsync(principal) as User)?.As<ContentItem>(UserDetails)?.As<UserDetailsPart>();
can here indeed be a shortcut as you don't need the instance of the User
type in this class.
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.
Same applies for the method above this one.
|
||
const string getInputsScript = @"return JSON.stringify( | ||
Array.from(document.querySelectorAll('form[action=\'/user/details\'] input, form[action=\'/user/details\'] select')) |
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.
You can also use backticks as string delimiter in JS (even if it's not a template string), so you won't run out of quote types:
Array.from(document.querySelectorAll('form[action=\'/user/details\'] input, form[action=\'/user/details\'] select')) | |
Array.from(document.querySelectorAll(`form[action='/user/details'] input, form[action='/user/details'] select`)) |
OCC-169
Fixes #313