Skip to content

[UI] Plan and Apply commands with additional options#510

Merged
mykalmax merged 25 commits intomainfrom
max/UI_plan_additional_options
Mar 16, 2023
Merged

[UI] Plan and Apply commands with additional options#510
mykalmax merged 25 commits intomainfrom
max/UI_plan_additional_options

Conversation

@mykalmax
Copy link
Contributor

@mykalmax mykalmax commented Mar 9, 2023

  • Include additional options when running plan
  • Include additional options when applying plan
  • Pass change category when applying plan
  • Set default dates when running plan first time
  • Disable and set to defaults not applicable additional options on initial run
  • Refactor Plan shared state into Context to keep state closer to the Plan component
  • Add hints and messaging based on different additional options

Screenshot 2023-0 3-09 at 11 06 05 AM
Screenshot 2023-03-09 at 11 07 38 AM
Screenshot 2023-03-09 at 11 06 33 AM

@mykalmax mykalmax force-pushed the max/UI_plan_additional_options branch 2 times, most recently from 2d2b212 to 933dc1f Compare March 9, 2023 18:59
@mykalmax mykalmax requested a review from a team March 9, 2023 19:09
@mykalmax
Copy link
Contributor Author

mykalmax commented Mar 9, 2023

Need some help to understand how to pass restate_models, currently sending from UI just a coma-separated string but plan commands expects t.Iterable[str]

@vchan
Copy link
Contributor

vchan commented Mar 9, 2023

You can use a Pydantic validator to parse the comma separated string into a list. So you'd use a Pydantic model for inputs, which means not using query parameters but a json post body, then add a validator for restate_models that parses the string to a list. It'd look something like this

@validator("restate_models")
def validate_restate_models(cls, v: str | list[str]) -> list[str]:
    if isinstance(v, str):
        return v.split(",")
    return v

@mykalmax mykalmax force-pushed the max/UI_plan_additional_options branch from 18370eb to cf3ed81 Compare March 10, 2023 21:03
@mykalmax mykalmax marked this pull request as ready for review March 10, 2023 21:05
@mykalmax mykalmax requested a review from vchan March 13, 2023 15:44
@mykalmax mykalmax force-pushed the max/UI_plan_additional_options branch 5 times, most recently from fb7905e to 6aa2d28 Compare March 16, 2023 18:24
Copy link
Contributor

@vchan vchan left a comment

Choose a reason for hiding this comment

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

Looking good on the server side.

Comment on lines 28 to 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work? I'd normally create a Pydantic model, e.g. ApplyInput, to encapsulate the POST body. It's how the FastAPI documentation usually does it as well.

Comment on lines 22 to 24
Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment above regarding using a Pydantic model.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use a more descriptive name. Maybe PlanOptions? And if you add ApplyInput and/or PlanInput, you can subclass PlanOptions. Or rename this to PlanInput and ApplyInput can extend it.

@mykalmax mykalmax force-pushed the max/UI_plan_additional_options branch from e6dad7f to 601896e Compare March 16, 2023 20:46
@mykalmax mykalmax merged commit a7d750a into main Mar 16, 2023
@mykalmax mykalmax deleted the max/UI_plan_additional_options branch March 16, 2023 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants