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

Update model.py #831

Closed
wants to merge 2 commits into from
Closed

Update model.py #831

wants to merge 2 commits into from

Conversation

Luci2015
Copy link
Collaborator

@Luci2015 Luci2015 commented Nov 20, 2023

Instead of going through the details of SettingsInfo statement of each group, just copy the entire statement from the Sign response

Fixes #830

@bhunut-adobe
Copy link
Collaborator

@Luci2015, We need to utilize the Class model that @adorton-adobe defined for settings.

reviewed fix for keeping class model as before and check if a boolean is sent over instead of a dict
_canUseMulticastWorkflows_ will not even appear in the final SettingsInfo class because of the _remove_unknown_keys_ function which limits results to:
    libaryDocumentCreationVisible: BooleanSettingsInfo = None
    sendRestrictedToWorkflows: BooleanSettingsInfo = None
    userCanSend: BooleanSettingsInfo = None
    userManagedWorkflowsEnabled: BooleanSettingsInfo = None
    allowedToShareUserCreatedWorkflows: BooleanSettingsInfo = None
@Luci2015
Copy link
Collaborator Author

@bhunut-adobe :
reviewed fix for keeping class model as before and check if a boolean is sent over instead of a dict
canUseMulticastWorkflows will not even appear in the final SettingsInfo class because of the remove_unknown_keys function which limits results to:
libaryDocumentCreationVisible: BooleanSettingsInfo = None
sendRestrictedToWorkflows: BooleanSettingsInfo = None
userCanSend: BooleanSettingsInfo = None
userManagedWorkflowsEnabled: BooleanSettingsInfo = None
allowedToShareUserCreatedWorkflows: BooleanSettingsInfo = None

@bhunut-adobe
Copy link
Collaborator

@Luci2015 Why not just add canUseMulticastWorkflows attribute to SettingsInfo class?

@ddybvig-adobe
Copy link
Collaborator

I tried adding the canUseMultiCastWorkflows field to SettingsInfo, but I still get the same error as before.

@Luci2015
Copy link
Collaborator Author

Luci2015 commented Nov 22, 2023

@bhunut-adobe : Andrew discarded also some other statements that come from the Sign response (last time it was widgetCreationVisible). It's the reason why he added the remove_unknown_keys function in the first place. Only the 5 above were kept for some reason.
I can add it, no issue, but the proposed solution should be sufficient to get back to the previous state of the tool -> your call

Copy link
Collaborator

@ddybvig-adobe ddybvig-adobe left a comment

Choose a reason for hiding this comment

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

Approved.

@ddybvig-adobe
Copy link
Collaborator

Hi @Luci2015 , it looks like Sign Engineering fixed this on stage as shown here. For now, I am going to leave this PR unmerged in hopes that they deploy this change to production soon. If a need does arise for a signed build and release of UST before then, let me know.

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.

Sign sync is failing
4 participants