Skip to content
This repository was archived by the owner on Jul 11, 2019. It is now read-only.

#654 - fix the showing of static pages with underscore#676

Open
miroslavt wants to merge 1 commit intoliqd:developfrom
miroslavt:develop
Open

#654 - fix the showing of static pages with underscore#676
miroslavt wants to merge 1 commit intoliqd:developfrom
miroslavt:develop

Conversation

@miroslavt
Copy link

The main fix is in routing.py. I moved '/static/{key}{.format}' up in the list so that the two rules with "_" do not mess it. The actions "edit" and "update" should not rely on underscores to match their URLs. Some other matching should be devised. This should be part of their implementation.

StaticController.serve() is a fix - "http://host/static/about" causes an error without it.
The other changes in StaticController are just for consistency. These methods are not used anyway.

@nidico
Copy link
Collaborator

nidico commented Dec 11, 2013

Thanks for the PR!

However the edit and update methods are actually functional when adhocracy.staticpage_backend is set to database in adhocracy.ini. This PR breaks them, so we can't merge it like that.

However you're completly right - the edit and update URLs are poorly designed, they shouldn't rely on the underscore to determine the chosen language. I'd rather suggest to use query parameters for that and change urls to /static/{key} (GET: serve, POST: update) and /static/{key}/edit (GET: edit), inline with proposals and such. It might look strange to use query (GET) parameters for the language in a POST request (update), but it'd work just fine. Alternatively the language could be put in a hidden field in the form.

Another small note: When fixing multiple things in one PR, I'd favor to use multiple commits (e.g. git-cola is nice to split new stuff, or use git add -p or git commit -p).

@miroslavt
Copy link
Author

I was thinking about asking you about edit() and update() but from what I saw in the code it seemed to me that they are not used. So I did the quick and dirty fix.

I have just activated the static page administration and I see that it is working quite well. I will go and make a real fix this time.

How about making the changes in this way. /static/{key}{.format} is reserved for serve() only. It stays last in the routing list. Then we devise a new URL format /static/{key}/edit/{lang}. GET and HEAD are for edit() and POST is for update(). This way the language is passed in the REST style and {.format} goes away from editing - there is no meaning in showing the editing form via overlay.

@nidico
Copy link
Collaborator

nidico commented Dec 16, 2013

I will go and make a real fix this time.

Great!

How about making the changes in this way. /static/{key}{.format} is reserved for serve() only. It stays last in the routing list. Then we devise a new URL format /static/{key}/edit/{lang}. GET and HEAD are for edit() and POST is for update().

Yep, I'm fine with this.

nidico pushed a commit that referenced this pull request Feb 17, 2014
Everything will be better once #676 is done.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants