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.

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

fix: don't include unenrolled channels and hubs when prompting user for edge:channels:unenroll #513

Merged
merged 1 commit into from May 15, 2023

Conversation

rossiam
Copy link
Collaborator

@rossiam rossiam commented May 11, 2023

  • when using prompts for unenrolling a hub from a channel
    • prompt for hub first, skipping hubs that aren't enrolled in any channels
    • only include enrolled channels when prompting for channel
  • added examples to edge:channels:unenroll since I was updating it
  • changes to library to support this
    • ChooseOptions type now includes optional listItems so chooseX methods can easily support alternate lists
    • ChooseOptions is now generic so listItems can require function that returns list of the correct things
    • specify the generic where ChooseOptions is used
    • updated chooseHubs and chooseChannel to support alternate listItems
    • unit tests for updated for chooseHubs and chooseChannel

Checklist

  • I have read the CONTRIBUTING document
  • Any required documentation has been added
  • My code follows the code style of this project (npm run lint produces no warnings/errors)
  • I have added tests to cover my changes

@changeset-bot
Copy link

changeset-bot bot commented May 11, 2023

🦋 Changeset detected

Latest commit: 7874446

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@smartthings/cli Patch
@smartthings/plugin-cli-edge Patch
@smartthings/cli-lib Patch
@smartthings/cli-testlib Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines 3393 to 3397
prompt user for a channel the given hub is enrolled in and then unenroll it

$ smartthings edge:channels:unenroll 2fa0912f-cb73-424e-97f9-ffff76ea4f2a

unenroll the hub from the specified channel
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the difference between these two examples? The first seems to be showing unenrolling a hub from a channel, and the second is "unenroll[ing] the hub from the specified channel".

Copy link
Contributor

Choose a reason for hiding this comment

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

The first example doesn't provide any arguments, so the user will be shown a list of hubs and prompted to choose one, then shown a list of channels that hub is subscribed to and prompted to choose one. The second example gives one argument -- the UUID of a hub -- so it'll skip straight to asking for the channel to unsubscribe that specific hub from. The third example specifies the UUID of both the hub and the channel to unenroll it from, so the user won't be prompted for anything else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the second example (last of the three), nothing is prompted for. In the first, the user doesn't specify a channel on the command line so they are prompted for one. I've updated the text to say "unenroll the specified hub from the specified channel".

@@ -14,6 +14,7 @@ import {
chooseOptionsDefaults,
chooseOptionsWithDefaults,
forAllOrganizations,
ListDataFunction,
Copy link
Contributor

Choose a reason for hiding this comment

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

GitHub Actions noticed that ListDataFunction isn't used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, fixed.

@rossiam rossiam merged commit 13167bd into SmartThingsCommunity:main May 15, 2023
4 checks passed
@rossiam rossiam deleted the unenroll-only-enrolled branch May 15, 2023 16:34
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.

None yet

3 participants