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

SAK-50557 LTI exceptions thrown for some tools placed in users workspace #12941

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hornersa
Copy link
Contributor

@hornersa hornersa commented Oct 1, 2024

Jira: https://sakaiproject.atlassian.net/browse/SAK-50557

Refer to the screencast attached to the jira for details about the proposed fix.

Also, the proposed CSS change helps better align the tool title of a deployed external tool with the other tool titles in a given site's tool menu. (Currently the icon elements with class fa are two pixels too wide compared to icons with class si.)

@hornersa hornersa requested a review from csev October 1, 2024 23:13
@ern ern changed the title SAK-50557 LTI: Exceptions thrown for some tools placed on Home site SAK-50557 LTI exceptions thrown for some tools placed in users workspace Oct 14, 2024
Comment on lines 322 to 323
log.debug("login_hint={}", login_hint);
log.debug("encoded_login_hint={}", encoded_login_hint);
Copy link
Contributor

@ern ern Oct 14, 2024

Choose a reason for hiding this comment

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

Suggested change
log.debug("login_hint={}", login_hint);
log.debug("encoded_login_hint={}", encoded_login_hint);
log.debug("login_hint=[{}], encoded_login_hint=[{}]", login_hint, encoded_login_hint);

Comment on lines 136 to 137
log.debug("encoded_login_hint={}", encoded_login_hint);
log.debug("login_hint={}", login_hint);

Copy link
Contributor

Choose a reason for hiding this comment

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

@hornersa you should combine these into a single log message and differentiate it from the similar debug statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume the class and method called (and recorded in the log statement) would distinguish one pair from another.

@hornersa
Copy link
Contributor Author

@ern - Later this morning I intend to push a new commit to incorporate your suggested revisions.

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.

2 participants