SAK-50378 LTI - Remove Tool Features / Rationalize DB Fields #12844
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This will need a rebase once #12851 is in master - so I have it draft for now. |
ern
reviewed
Sep 5, 2024
Comment on lines
+2
to
+33
| -- These are columns dropped by SAK-50378 but will not be droppped by | ||
| -- the Sakai 25 conversion scripts until Sakai 25.2 or later | ||
| -- New Sakai 25 instances won't have these columns but the columns will | ||
| -- not be removed as systems go from Sakai 23 to Sakai 25.0 or 25.1 to allow | ||
| -- for forward and backward version movement for early versions of Sakai 25 | ||
|
|
||
| ALTER TABLE lti_tools DROP COLUMN | ||
|
|
||
| ALTER TABLE lti_content DROP COLUMN pagetitle; | ||
| ALTER TABLE lti_content DROP COLUMN toolorder; | ||
| ALTER TABLE lti_content DROP COLUMN consumerkey; | ||
| ALTER TABLE lti_content DROP COLUMN secret; | ||
| ALTER TABLE lti_content DROP COLUMN settings_ext; | ||
| ALTER TABLE lti_content DROP COLUMN fa_icon; | ||
|
|
||
| ALTER TABLE lti_tools DROP COLUMN allowtitle; | ||
| ALTER TABLE lti_tools DROP COLUMN pagetitle; | ||
| ALTER TABLE lti_tools DROP COLUMN allowpagetitle; | ||
| ALTER TABLE lti_tools DROP COLUMN allowlaunch; | ||
| ALTER TABLE lti_tools DROP COLUMN allowframeheight; | ||
| ALTER TABLE lti_tools DROP COLUMN allowfa_icon; | ||
| ALTER TABLE lti_tools DROP COLUMN toolorder; | ||
| ALTER TABLE lti_tools DROP COLUMN allowsettings_ext; | ||
| ALTER TABLE lti_tools DROP COLUMN allowconsumerkey; | ||
| ALTER TABLE lti_tools DROP COLUMN allowsecret; | ||
|
|
||
| -- Also while not a database conversion per se, there is an undocumented property that causes | ||
| -- The LTI service to derive or fake most of these values when tools are retrieved from the DB | ||
| -- post Sakai 25. This is defaulted to "false" for testing and hopefuly it will not be needed | ||
| -- and can be deleted at Sakai 25.2 or later | ||
|
|
||
| sakai-sak-50328-fake-deprecated-values=true |
There was a problem hiding this comment.
I would remove this txt file and PR to sakai-reference
There was a problem hiding this comment.
Also lets not talk about versions when things happen because in my experience they just end up changing.
ern
reviewed
Sep 5, 2024
basiclti/basiclti-common/src/java/org/sakaiproject/util/foorm/Foorm.java
Outdated
Show resolved
Hide resolved
ern
reviewed
Sep 5, 2024
basiclti/basiclti-common/src/java/org/sakaiproject/util/foorm/Foorm.java
Outdated
Show resolved
Hide resolved
ern
reviewed
Sep 5, 2024
basiclti/basiclti-common/src/java/org/sakaiproject/util/foorm/Foorm.java
Show resolved
Hide resolved
ern
reviewed
Sep 5, 2024
basiclti/basiclti-common/src/java/org/sakaiproject/util/foorm/Foorm.java
Outdated
Show resolved
Hide resolved
ern
reviewed
Sep 5, 2024
basiclti/basiclti-common/src/java/org/sakaiproject/util/foorm/Foorm.java
Outdated
Show resolved
Hide resolved
ern
reviewed
Sep 5, 2024
basiclti/basiclti-impl/src/java/org/sakaiproject/lti/impl/BaseLTIService.java
Outdated
Show resolved
Hide resolved
ern
reviewed
Sep 5, 2024
Comment on lines
+179
to
+182
| // SAK-50378 - If requested (i.e. we found something wrong after Sakai-25 is released), we fake up deprecated values | ||
| // This is quicker than a patch and can be used while a patch is being developed and/or for | ||
| // testing to see if the deleted columns caused a regression | ||
| // This entire block of code can be removed after Sakai 25.2 if it is not needed by then |
There was a problem hiding this comment.
lets not discuss Sakai versions in code
ern
reviewed
Sep 5, 2024
| // This is quicker than a patch and can be used while a patch is being developed and/or for | ||
| // testing to see if the deleted columns caused a regression | ||
| // This entire block of code can be removed after Sakai 25.2 if it is not needed by then | ||
| String ltiFakeDeprecated = serverConfigurationService.getString("sakai-sak-50328-fake-deprecated-values", "false"); |
There was a problem hiding this comment.
Sure - I will leave it in and we can review at the Tuesday meeting. I will fix the indentation tho :)
…Foorm.java Co-authored-by: Earle Nietzel <earle@longsight.com>
…Foorm.java Co-authored-by: Earle Nietzel <earle@longsight.com>
…Foorm.java Co-authored-by: Earle Nietzel <earle@longsight.com>
…LTIService.java Co-authored-by: Earle Nietzel <earle@longsight.com>
…Foorm.java Co-authored-by: Earle Nietzel <earle@longsight.com>
csev
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This need to land after Zhuo's JIRA 50427 lands. I will rebase this once his code is in master. But folks can look at it now.