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

Updated to check if a public link to use the organization website instead of / #1247

Merged
merged 3 commits into from Oct 8, 2022
Merged

Conversation

ghost
Copy link

@ghost ghost commented Oct 6, 2022

If I am using this with a client, I don't want them to be able to click on my logo and go to my public dashboard. Not like they could login, but I feel like it should link to the organization website since they can't do anything in the backend (the point of the public link).

This is my first PR with WebODM. In addition, I am not an expert in Django, so if there's an easier or more "Django-y" way to do this, please let me know. Thank you!

@pierotofy
Copy link
Member

pierotofy commented Oct 7, 2022

Hey @ssantacroce-uwach 👋 thanks for the PR! As not everyone who uses WebODM sets, or even has, an organization website (it's blank by default), perhaps the conditional check should include for the presence of the organization website variable (if it's blank, redirect to /, else redirect to the organization website for on public urls)?

@ghost
Copy link
Author

ghost commented Oct 7, 2022

So I could be mistaken, but with a new system the organization_website gets set to either the WebODM or OpenDroneMaps page. It is not blank.

But if that is a concern, would a check that the organization_website not being blank suffice?

@pierotofy
Copy link
Member

pierotofy commented Oct 7, 2022

Ah, good point; it's actually set to https://github.com/OpenDroneMap/WebODM/.

I guess a good way to do this would be to move the default value to a variable, then expose that variable to the django view (or make a django function) and check to see if the value has been changed from the default. (Don't use "https://github.com/OpenDroneMap/WebODM/" in the view's if expression, that would be a duplication).

@ghost
Copy link
Author

ghost commented Oct 7, 2022

I just modified it to go to '/' if the org url is blank. It really shouldn't matter if the url is the default. If you're on the "public" link, it shouldn't have a url that goes back to the dashboard, because that isn't something a public viewer would be able to login to. And if it's a logged in user for whatever reason viewing the public link, they know how to get back to the dashboard...

@pierotofy
Copy link
Member

I got it; how about checking if the user is logged in?

  • Public URL and anonymous? --> org website (or "/" if blank)
  • Else --> "/"

@ghost
Copy link
Author

ghost commented Oct 7, 2022

That is a way better idea! Sorry I'm very new to Django, my main framework is FastAPI. Your suggestion has been implemented!

@pierotofy pierotofy merged commit 01373dc into OpenDroneMap:master Oct 8, 2022
@pierotofy
Copy link
Member

Awesome, thanks!!

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.

None yet

1 participant