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

Make Canvas Integration work with new UI #1749

Conversation

edalex-ian
Copy link
Member

@edalex-ian edalex-ian commented May 20, 2020

Checklist

Description of change

Solves #1744. The problem was that the LegacyContentApi did not set up the form correctly in the returnResult method, and the frontend wasn't expecting it to contain the form snippet that caused content to be submitted to Canvas.

Note: This is a hack, and should be done more elegantly.
We should not be using dangerouslySetInnerHTML.
@edalex-ian
Copy link
Member Author

We're really just looking for a green codebuild - as it's running in New UI mode.

Rather than using security risk dangerouslySetInnerHtml.
Also - remove test strings from LegacyContentApi.scala
and use idiomatic Scala code to setup the form.
@SammyIsConfused
Copy link
Contributor

@edalex-ian All feedback has been addressed.

@SammyIsConfused SammyIsConfused requested a review from a team May 21, 2020 05:23
@edalex-ian
Copy link
Member Author

Heya @SammyIsConfused ,

Before we all now do the proper review we'll want it point to the hotfix branch or the like.

@SammyIsConfused SammyIsConfused changed the base branch from master to hotfix/2020.1.2 May 21, 2020 05:27
@SammyIsConfused SammyIsConfused marked this pull request as ready for review May 21, 2020 05:31
@edalex-ian
Copy link
Member Author

Looks okay to me, but seeing I raised it I can't explicitly approve.

I do wonder if we can improve the Option/Some stuff - seems rather verbose - but that's okay. :)

@edalex-ian
Copy link
Member Author

Oh @SammyIsConfused , don't forget we also need to 'forward' port this onto develop. (The process has definitely been skewed for this rush.

@SammyIsConfused SammyIsConfused merged commit 6c7b710 into openequella:hotfix/2020.1.2 May 22, 2020
SammyIsConfused added a commit that referenced this pull request May 22, 2020
…round

Make Canvas Integration work with new UI
SammyIsConfused added a commit that referenced this pull request May 22, 2020
…round

Make Canvas Integration work with new UI
SammyIsConfused added a commit that referenced this pull request May 28, 2020
…round

Make Canvas Integration work with new UI
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.

4 participants