evildmp / Arkestra

Arkestra extends Django CMS to provide an intelligent semantic web publishing system for organisations and institutions.

http://arkestra-project.org

BSD 2-Clause "Simplified" License

139 stars 27 forks source link

Dry url issue69 #71

Closed darbula closed 11 years ago

darbula commented 11 years ago

This pull request replaces pull/issue #69 as that issue seems to have some work ahead and by my mistake it did not have a dedicated named branch. Now I have made a dedicated branch issue69 to be used to fix everything concerning following problem:

Description:

Urls for contacts, news and vacancies were defined in multiple places:

urls.py,
models.py - in url_path and
menus.py - in url_attribute.

This situation prevents changing/translating the default urls (i.e. contacts/ news/ etc). In this commit url patterns in urls.py are given names and then reverse is used to resolve urls where needed i.e. in Entity.get_related_info_page_url or in URLModelMixin.get_absolute_url.

Url patterns are named so that their name can be deduced from url_path and url_attribute when needed.

evildmp commented 11 years ago

I'd like to do something with some of these ideas, but I will need to discuss it first. One issue is that it is important that it should be easy to provide Entities with connections to other kinds of information through other applications.

For example, our Publications application makes it possible to see an Entity's academic research publications, and I am not sure how that will be affected by these changes.

darbula commented 11 years ago

There should not be any consequences to external applications but if you can provide some more details about certain parts that could become problematic we can discuss it.

evildmp commented 11 years ago

I'm taking a good look at this now. There are some changes in the develop branch since you made this pull request, so I will reconcile those first, and make some a reminder to myself below about things that need tests and/or comments to explain their behaviour.

tests/comments required

Person.get_absolute_url() - test for self.active
basic tests for news and events, vacancies and studentships views
tests for get_absolute_url() methods where not present

evildmp commented 11 years ago

Have a look at https://github.com/evildmp/Arkestra/tree/merge-darbula-issue69. This:

incorporates your changes
rationalises the naming of reverse patterns
rationalises and reformats URL patterns
includes tests for reverse patterns

I will do some more testing, and if this looks OK it's ready to be merged.

evildmp commented 11 years ago

Merged in #106 - thanks.

darbula commented 11 years ago

Great, glad that this has worked its way into upstream :)