Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(ui): ability to create a bucket from time machine #17860

Merged
merged 13 commits into from
Apr 27, 2020
Merged

feat(ui): ability to create a bucket from time machine #17860

merged 13 commits into from
Apr 27, 2020

Conversation

alexpaxton
Copy link
Contributor

@alexpaxton alexpaxton commented Apr 24, 2020 •
edited
Loading

This change makes creating a bucket much more natural by enabling it without leaving the time machine

create-bucket-on-the-fly

  • Refactor CreateBucketOverlay to get everything it needs from redux and dispatch
  • Add create-bucket to overlay controller
  • Create a component within BucketSelector that shows the create bucket form within a popover
    • This is disabled if a cloud user has maxed out buckets
  • Refactor bucket selector in time machine to get list directly from redux instead of getActiveTimeMachine
  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Well-formatted commit messages
  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Documentation updated or issue created (provide link to issue/pr)
  • Signed CLA (if not already signed)

@alexpaxton alexpaxton requested review from a team and drdelambre and removed request for a team April 24, 2020 19:57
@drdelambre
Copy link
Contributor

this is 💯 amazing and blows my mind and delivers all of my hopes and dreams, but write a cypress test for it. "i went to data explorer, clicked on a button, filled out a name and clicked on some more buttons, and then it was there forever"

121watts
121watts reviewed Apr 24, 2020
View reviewed changes
ui/src/timeMachine/components/SelectorListCreateBucket.tsx Outdated
const [bucketRetentionRules, setBucketRetentionRules] = useState<any>(
isRetentionLimitEnforced ? DEFAULT_RULES : []
)
const [bucketRuleType, setBucketRuleType] = useState<'expire' | null>(
Copy link
Contributor

Choose a reason for hiding this comment

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

There's enough state in here to justify moving this into a useReducer hook. It could all even stay in this file but would pull a bunch of this state management into a reducer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A similar thought I had was to make a component that is just the contents of the popover, then most of the state can live in there. We'd still have a lot of state though. The component I based this off of just had a whole Bucket object as a single piece of state so I flattened it out

@alexpaxton
Copy link
Contributor Author

@drdelambre I added the e2e test

@alexpaxton
Copy link
Contributor Author

@121watts alright I rebuilt the component with useReducer instead of useState check it out

@alexpaxton alexpaxton changed the title feat(ui): ability create a bucket from time machine feat(ui): ability to create a bucket from time machine Apr 27, 2020
@alexpaxton alexpaxton merged commit d56966f into master Apr 27, 2020
@alexpaxton alexpaxton deleted the feat/create-bucket-on-the-fly branch April 27, 2020 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@121watts 121watts 121watts left review comments

@drdelambre drdelambre drdelambre approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexpaxton @drdelambre @121watts