Generic dialog to create new request #22
Conversation
sponkrashin
changed the base branch from
develop
to
feature/dialog-with-requests-types
sponkrashin
changed the base branch from
feature/dialog-with-requests-types
to
develop
Duveshka
changed the base branch from
develop
to
feature/dialog-with-requests-types
Duveshka
changed the base branch from
feature/dialog-with-requests-types
to
develop
| } | ||
|
|
||
| public static HeroCard GetChoiceCard() | ||
| private static HeroCard ChoiceCard() |
There was a problem hiding this comment.
Method name should contain verb.
| return false; | ||
| } | ||
|
|
||
| if (formData["Button"].ToString() == Cancel) |
There was a problem hiding this comment.
Constant instead of magic string.
| { | ||
| GetInfoCard().ToAttachment() | ||
| additionalFieldsValues.Add(additionalFieldsData[i].Last.ToString()); |
There was a problem hiding this comment.
What is type of additionalFieldsData[i]?
| return await stepContext.PromptAsync(nameof(TextPrompt), | ||
| new PromptOptions | ||
|
|
||
| if (string.IsNullOrEmpty(formData["Title"].ToString()) |
|
|
||
| var createRequestTypeFields = | ||
| (await this.mediator.Send(new GetServiceDeskRequestTypesQuery(), cancellationToken)) | ||
| .First(requestTypeDTO => requestTypeDTO.Id == Convert.ToInt32(formData["TypeId"].ToString())) |
| } | ||
|
|
||
| var createRequestTypeFields = | ||
| (await this.mediator.Send(new GetServiceDeskRequestTypesQuery(), cancellationToken)) |
There was a problem hiding this comment.
Move getting of request type fields to separate method.
|
|
||
| var newRequest = new CreateRequestDTO | ||
| { | ||
| Title = formData["Title"].ToString(), |
| switch (field.FieldType) | ||
| { | ||
| case RequestTypeUIFieldType.Year: | ||
| textBlock = new AdaptiveTextBlock("Enter a year"); |
There was a problem hiding this comment.
Year of what? A field name should be added somewhere.
| var promptOptions = new PromptOptions | ||
| { | ||
| Prompt = (Activity)MessageFactory.Attachment(InputFormCard(cardBody, Actions)), | ||
| RetryPrompt = MessageFactory.Text("Not all fields are filled. Repeat") |
| { | ||
| Title = "In Development", | ||
| Buttons = new List<CardAction> | ||
| return await stepContext.ReplaceDialogAsync(nameof(MainDialog), null, cancellationToken); |
There was a problem hiding this comment.
Probably, it is better to send user to previous dialog instead of main dialog in such case.
| }; | ||
| } | ||
|
|
||
| private static Attachment InputFormCard(List<AdaptiveElement> body, List<AdaptiveAction> actions) |
| private const string Cancel = "Cancel"; | ||
| private const string Yes = "Yes"; | ||
| private const string No = "No"; | ||
| private const string Button = "Button"; |
There was a problem hiding this comment.
I suppose it should be the same as property name in line 200. In that case you'd better introduce new private interface like interface IButtonData { string Button { get; set; } } and use nameof(IButtonData.Button). Then you will never forgot to change value in one place after changing in another.
|
|
||
| new AdaptiveTextInput { Id = TypeId, Value = requestTypeDTO.Id.ToString(), IsVisible = false }, | ||
|
|
||
| new AdaptiveTextBlock(Title), |
There was a problem hiding this comment.
You don't need to use constants everywhere or at least you need to separate constants used for ids to prevent misprints when accessing form values from constants used to show user in UI.
You use title of text block and it's input placeholder only here, you don't need to access them later from anywhere - thus you don't have to use constant.
| private const string Description = "Description"; | ||
| private const string ExecutionDate = "Execution Date"; | ||
| private const string Username = "[email protected]"; | ||
| private readonly IRequestTypeUIFactory requestTypeUIFactory; |
There was a problem hiding this comment.
Please, add empty line between constants and fields.
| { | ||
| var attachments = new[] | ||
| var formData = (JObject)promptContext.Context.Activity.Value; |
There was a problem hiding this comment.
You use same code thrice - to validate, in choice step and to get final values. It could be better to create internal class which will represent form data and return parsed form values from a method.
No description provided.