Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Template paths and redirects #298

Merged
merged 11 commits into from
Mar 28, 2022
Merged

Template paths and redirects #298

merged 11 commits into from
Mar 28, 2022

Conversation

s-cork
Copy link
Collaborator

@s-cork s-cork commented Mar 27, 2022

@jshaffstall

attempt number 2.

Maybe you could have a play with it by cloning this version of anvil-extras:
https://anvil.works/build#clone:6ATPRINDTZUXBKSQ=R3XKGGOVQZK6MZVHG36HCTLU

Then changing the dependency to the local copy while we still work out the kinks.

I changed a bunch of logging statements as I was working on it to make it easier to see what was going on.

The api is the one we came up with on the other branch:

@routing.redirect(...) # same args as template
def redirect_no_admin():
    # routing.set_url_hash("login", ...)
    # or return a url_hash
    return "login"

@s-cork s-cork force-pushed the template-paths branch 2 times, most recently from 2f44571 to 9081f37 Compare March 27, 2022 10:34
@jshaffstall
Copy link
Contributor

Not sure what is wrong with this: https://anvil.works/build#clone:KFF6KEV4NDMJJK5U=JI5EORBERTEDFD7AG4O5WTGW

When I run it under the regular development version of anvil_extras, it shows the public home page when run. When run under the clone version it does not show the home page. I've commented out the redirect and login template for now.

@s-cork
Copy link
Collaborator Author

s-cork commented Mar 27, 2022

oh sugar - that should be fixed now - trying to prevent an infinite loop caused the first loop to not execute!

Fixed -

if you're git savy you can clone the anvil-extras-dev locally
clone this repo locally
and do some git incantations rather than recloning

@jshaffstall
Copy link
Contributor

Uncommented out the redirect and login router.

The login router intercepts correctly when trying to go to any of its routes.

The first run I got an error about self.current not existing in login_click, but after that one run it worked fine. Not sure what that was about.

The redirect decorator works great, non-admins are getting redirected to #user.

As far as I can tell from the setup I have, it all seems to be working fine. This technique reduces the number of template conditions needed by quite a bit. I only have conditions on my login router and my no admin redirect. By the time it gets to the user or admin routers, I already know the user is logged in and is an admin if they're trying to go to the admin pages.

@s-cork s-cork changed the title Template paths Template paths and redirects Mar 27, 2022
@s-cork
Copy link
Collaborator Author

s-cork commented Mar 27, 2022

So we're calling this branch a win.

I'll give it another once over later today
add some docs
and it'll be ready for review

side note - i see you're using/experimenting with Tabulator.
Definitely move to version v2.1.0 if you can - it's recently gone through a rewrite.

and my bad for using replace_current as a kwarg, it should be replace_current_url everywhere.

@jshaffstall
Copy link
Contributor

Yeah, I wrote another sample app that included some pages and sidebar links, and everything worked as expected. Definitely a win.

@s-cork s-cork marked this pull request as ready for review March 28, 2022 07:01
@s-cork s-cork requested a review from meatballs March 28, 2022 07:01
@meatballs meatballs merged commit af99ae8 into main Mar 28, 2022
@meatballs meatballs deleted the template-paths branch March 28, 2022 07:42
@s-cork
Copy link
Collaborator Author

s-cork commented Mar 28, 2022

@jshaffstall this is now on the development branch
keep us posted on anything you notice in your experiments!

@s-cork s-cork restored the template-paths branch April 13, 2022 05:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants