Fix(GH) : account token component not finding correct account #3172
Conversation
connorivy
requested review from
clairekuang,
AlanRynne,
teocomi and
jozseflkiss
as code owners
| "Account to get the auth token from. Expects the `userId`", | ||
| GH_ParamAccess.item | ||
| ); | ||
| pManager.AddParameter(new SpeckleAccountParam()); |
There was a problem hiding this comment.
This changes the input parameter type, which would be a breaking change for users with GH scripts with this component that are passing in a text parameter. Are we okay with this @AlanRynne ?
There was a problem hiding this comment.
Nevermind, looks like the input node was an Accounts node which previously passed the user id as the output now passes the Account object. Looks good to go ✌️
There was a problem hiding this comment.
Just triple checked that this is fine. Old files continue to work with the new changes, although there is a small caveat.
There was a problem hiding this comment.
These changes do the trick except for updating both name and description of the account input. Built with this branch, i tested an opening a file with an existing "Get Token From Account" component
While the a newly placed component will look like this:
I'm not sure this is a big enough issue to request changing this.
The solution would involve:
- Leaving the original component intact and flag it as obsolete
- Creating a brand new component with these changes
- Create a class that implements IGH_UpgradeObject to define what is involved in "upgrading" an existing component to the new one.
This would also have the side-effect of forcing the users to upgrade their existing nodes in each file (which is Grasshopper recommended approach), which I'm not sure it's worth it as this will only affect descriptions and names of pre-existing components, but the components will work as expected
| "Account to get the auth token from. Expects the `userId`", | ||
| GH_ParamAccess.item | ||
| ); | ||
| pManager.AddParameter(new SpeckleAccountParam()); |
There was a problem hiding this comment.
Just triple checked that this is fine. Old files continue to work with the new changes, although there is a small caveat.
There was a problem hiding this comment.
Approving after discussion during standup.
Opened ticket to polish off the UI quirks in my comment https://spockle.atlassian.net/browse/CNX-8923
Description & motivation
Account token component was broken by this PR #3141 which changed the behavior of accounts
This PR applies the same fix that was used for the other components that depended on the account component.
Changes:
To-do before merge:
Screenshots:
Validation of changes:
Checklist:
References