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

Add view_on_site method in course admin (#1411) #1434

Merged
merged 1 commit into from
Nov 15, 2022

Conversation

ssciolla
Copy link
Contributor

This PR aims to resolve #1411.

@jonespm
Copy link
Member

jonespm commented Nov 1, 2022

I was testing this PR and it looks like this does work in production. This might just be a a misconfiguration on localhost with the sites table.

It looks like this fix you have here also works and goes directly to the link but whatever Django is doing to redirect is also working if the sites table is defined correctly. I think either of these is probably fine since this is a simple change.

@ssciolla
Copy link
Contributor Author

Ah, you are correct, @jonespm. If I change it to my ngrok URL, or add my URL to ALLOWED_HOSTS, then it works as expected. Is my change here worth adding? Not sure...

@jonespm
Copy link
Member

jonespm commented Nov 14, 2022

It's a small change, I'm fine merging it as it should work in all cases.

Otherwise we'd have to document to make sure the sites table is correctly configured. It looks like it should automatically configure it if the DOMAIN is set? I don't actually see DOMAIN in any of the sample configuration files though and not sure if I ever set it?

If we only ever set one site I'm not sure if we need the sites framework? Or maybe it would be useful if/when we put the LTI config in the database. Though CAE isn't using Sites.

@ssciolla
Copy link
Contributor Author

DOMAIN gets populated from ALLOWED_HOSTS in env.hjson. I think we may need the sites framework, but not 100 percent positive.

@ssciolla ssciolla merged commit 9ca3f27 into tl-its-umich-edu:master Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"View on Site" not working in Django Admin
2 participants