-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add commands to rekey device and create packages #509
Conversation
1. Added support to show preview of screenshot 2. Fixed a linter issue
Added support for the following commands: 1. Rekey Device: Rekey specified device based on the signing password ("brightscript.remoteControl.signingPassword") and package file ("brightscript.remoteControl.signedPackagePath") provided. 2. Create Package: This will present user a list of available launch configs to choose from. Once selected it will create a .zip file and .pkg file based on the config and will save them to the ${workspacefolder}/out 3. Rekey Device and Create Package: This will first rekey device and then create package for the selected launch config
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.
Hey, after I started looking over this, I realized we need to tweak the design a bit. We should sync up about this so I can explain in more detail. But we shouldn't use the "last used device" stuff like we did with the screenshot logic. Here's a rough overview of how the flow should work:
rekey:
a) "How do you want to enter your stuff?"
- "Pick from json file"
- "Enter manually"
b) prompt for host (prepopulated from json above if possible)
c) Prompt for password (prepopulated from json above if possible)
d) Prompt for signed package path (prepopulated from json above if available)
package:
a) "What should we package?"
- "Pick a folder"
- "Pick a zip"
- "Pick from a launch.json"
- show a launch.json option picker
- "Pick a rokudeploy.json"
-
b) prompt for host (prepopulated from json above if possible)
c) Prompt for password (prepopulated from json above if possible)
d) Prompt for signed package path (prepopulated from json above if available)
e) Show a summary of stuff, ask to click "Yes, do it" or "No, something's wrong, cancel".
rekeyAndPackage: same as "package" but do the rekey first, no new input necessary
src/BrightScriptCommands.ts
Outdated
await this.getRemoteHost(); | ||
await this.getRemotePassword(); | ||
await this.getSigningPassword(); | ||
await this.getSignedPackagePath(); |
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.
We should show a file picker for this.
Updates based on below reqs: rekey: a) "How do you want to enter your stuff?" - "Pick from json file" - "Enter manually" b) prompt for host (prepopulated from json above if possible) c) Prompt for password (prepopulated from json above if possible) d) Prompt for signed package path (prepopulated from json above if available) package: a) "What should we package?" - "Pick a folder" - "Pick from a launch.json" - show a launch.json option picker - "Pick a rokudeploy.json" - b) prompt for host (prepopulated from json above if possible) c) Prompt for password (prepopulated from json above if possible) d) Prompt for signed package path (prepopulated from json above if available) e) Show a summary of stuff, ask to click "Yes, do it" or "No, something's wrong, cancel".
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 have a few suggestions:
-
This stuff in this file getting a bit large, so let's move all this new logic out of BrightScriptCommands.ts into a separate src/commands/RekeyAndPackageCommands.ts file.
-
There seem to be a few bugs around picking a json file for the "Rekey device" flow. When I pick a rokudeploy.json with only
rekeySignedPackage
,devId
,signingPassword
,convertToSquashFS
, it fails with an error "A system error occurred (getaddrinfo ENOTFOUND :80). I'm assuming it's because the logic currently assumes that everything was supposed to be included in the json. That's not how it should work. We should load any values we find in the json, but then still walk through the manual prompts to allow the user to confirm those are the values they want, and fill in any missing values. -
When running "Rekey Device", and choosing "enter manually". after I enter my signing password, it just pops up a new file picker dialog, but there was no messaging around WHAT I'm supposed to pick. I think we need another intermediary dropdown with instructions saying "pick your pkg" so they know why the file picker appears.
-
After running the manual rekey for our internal app, I get this error at the end.
Most of the suggestions are addressed in the update. However i was not able to reproduce the error in #4 you mentioned |
@fumer-fubotv I finally got a chance to look over this. I've pushed some changes to optimize the flow a bit....somehow I had these sitting in an uncommited branch since November! The biggest change here is that we really need to prompt the user for all the details every time (either from json or from input boxes). We can't use values like the last password or last host used, because that might not be the device they want to rekey. One thing we need to tweak, is for the file picker modal, we should add information showing the currently selected zip (if there is one from rokudeploy.json), and add another button in that case saying "I want to change it". I see it having 2 flows:
It would be great if you could run through this workflow a few times to test it out, and iron out any rough edges. Then It should probably be good to go. |
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.
Here are a few things to change:
-
when choosing "I want to change something", it's not showing the previously-selected host in the host picker. That
userInputManager.promptForHost()
function accepts a default value. We should pass inrekeyConfig?host ?? defaultValues?.host
-
the "create package" flow doesn't remember the previously selected "what do you want to package" option when I choose "I want to change something".
-
the "rekey device and create package" command feels quirky. It seems like it just runs the rekey command and then the create package command next. It should collect all user input first (for rekey AND packaging inputs), and THEN show a summary at the end including all rekey inputs AND package/sign inputs. At that point, run both actions.
Host and create package flows have been updated as per feedback |
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.
Found a few more issues.
-
For "Create package" and choosing "use folder", it expects there to be a zip. But my folder has the source files, not a zip. We need to zip that folder as part of this flow.
-
For "Create Package", when I choose "Use a rokudeploy.json" file, and if that file doesn't have
rootDir
specified, the packaging fails.
-
For "Create package", when choosing "use launch.json", it appears to grab all the right values, but then the flow fails
I tried to reproduce the scenarios mentioned above and below are the findings:
|
1. updated outfile name to be the source folder name only 2. Fixed a bug for outdir that only supported mac paths 3. Added title to all inputboxes 4. Added a prompt for rootDir incase it is missing during create package 5. Added outfile path in dialog upon successful packaging
Added support for the following commands:
Rekey Device: Rekey specified device based on the signing password ("brightscript.remoteControl.signingPassword") and package file ("brightscript.remoteControl.signedPackagePath") provided.
Create Package: This will present user a list of available launch configs to choose from. Once selected it will create a .zip file and .pkg file based on the config and will save them to the ${workspacefolder}/out
Rekey Device and Create Package: This will first rekey device and then create package for the selected launch config
Here's a demo of the "Rekey Device" command:
rekey.mp4