Skip to content

Conversation

o-zander
Copy link
Contributor

@o-zander o-zander commented Jul 7, 2013

This is a very basic way to extend the page and the title model. I use it in conjunction with custom toolbar menu items. With some menu modifiers I hook the models to the menu to display an icon and a short description for every menu node. I decided to make this pull request to raise a discussion about this topic. A better support for page and title model extensions would be desirable. My approach could be easily integrated deeper in the page publish mechanism but I wanted to seperate it the most possible way to be independant from the current development process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this should be copy_title_extensions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right ... too much copy & paste used ;)

@yakky
Copy link
Member

yakky commented Jul 7, 2013

On a general level I like it.
Still undecided if this should be in core or not.
Being in core will ensure in will stay consistent with the rest of the codebase, but it will require maintance.
Taking it out of core will ease the burden of the CMS but with higher risks of bit rotting.

Anyhow documentation and thorough tests are required.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of whitespace issues in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes sorry, this file didn't belong to this topic. But anyway the file uses tabs for indentation and I use 4 spaces. Isn't the coding rule 4 spaces?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use tabs for html, js and css/sass

@digi604
Copy link
Contributor

digi604 commented Jul 8, 2013

I need docs with a basic example on how to do it.

@o-zander
Copy link
Contributor Author

o-zander commented Jul 8, 2013

I'm currently at work but I could write a doc this evening. I never wrote a doc before, can you give me a hint on where and how to do this?

@digi604
Copy link
Contributor

digi604 commented Jul 8, 2013

have a look in the docs folder... probably docs/extending_cms/app_integration.rst would be a good place.

@o-zander
Copy link
Contributor Author

o-zander commented Jul 8, 2013

I wrote a small doc on extending the page model. Placed it in a new file cause it isn't sure whether the extension makes it to the core or not ;) I had the time to test the extensions today and I noticed some things that have to be improved. For example I tried to write a MultiExtension with a ForeignKey instead of an OneToOne relation to have the possibility to add any number of that extension to the page. Or a problem with custom OneToOne relations on the extension. I'm currently need the extensions for a project I'm working on so I will test them throughout the week.

@digi604
Copy link
Contributor

digi604 commented Jul 9, 2013

Is there a way we could automate adding a toolbar entry for the extension admin?

@digi604
Copy link
Contributor

digi604 commented Jul 9, 2013

And yes i want this in core. :)

@o-zander
Copy link
Contributor Author

o-zander commented Jul 9, 2013

If this should go to core we could discuss about adding it to the page model directly. Extensions could then be copied in the process of publishing like the title model without the overhead of the signal.

Should there be one toolbar entry for every extension or an entry that shows all extensions? Perhaps a submenu that lists all extension admins?

I'm currently thinking about using two db tables for every extension. One for the draft and one for the live version, cause in my case I need a one to one relation with uniqueness on the extension. But thats impossible on one table. Do the plugins support one to one relations? I can't imagine that.

@yakky
Copy link
Member

yakky commented Jul 11, 2013

Linked to #1904

@o-zander
Copy link
Contributor Author

Hi guys, was busy this week sorry. I will review the linked discussion and hopefully get it final this weekend :)

@digi604
Copy link
Contributor

digi604 commented Jul 19, 2013

looks nice :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please read http://docs.django-cms.org/en/2.4.0/contributing/contributing.html#contributing-documentation and use the title style described there. thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the abstract model is located at cms.extensions.models.PageExtension not cms.models.pageextensionmodel.PageExtension

@o-zander
Copy link
Contributor Author

Short information: the work isn't finished yet, I still want to add an admin for the extensions. The docs will get rewritten and will match the doc conventions ;) Tests are still missing. Hope to get this done this week.

@stefanfoulis
Copy link
Contributor

thanks @o-zander :-)
I've been testing/reviewing your pull-request. We need this feature for a project at divio really soon. Would you mind if I continue development today?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are these changes necessary? They don't seem specific to this pull-request.

@o-zander
Copy link
Contributor Author

sorry for my late reply. No I don't mind ;) I'm glad I could help you in any way. I already used it in a production enviroment and could say its quite stable. I use a toolbar entry that adds a page extension and can open a admin modal. Important that no extension for the public page is created by hand or any other function. Else the save mechanism will break.

a word to the unnecessary changes: yes they doesnt belong to this pull request but I doesn't know how to restrict a pull request to spefic commits

@stefanfoulis
Copy link
Contributor

@o-zander yeah, it's looking pretty good. I didn't have to change much up to now.

To keep commits tidy for the pull requests, it's best to make feature branches for each pull-request, instead of using the develop branch.

@o-zander
Copy link
Contributor Author

Thanks, I will follow the advice the next time. Thought there must be a simpler way than cluttering your repo with dozens of branches ;) A feature that github could improve ...

@stefanfoulis stefanfoulis mentioned this pull request Jul 31, 2013
12 tasks
@stefanfoulis
Copy link
Contributor

merged in #2110

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.

6 participants