# Radiant Core Dev: Page Controller

**URL:** <https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711>\
**Category:** Radiant CMS\
**Created:** [June 1, 2007, 5:05pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711 "2007-06-01T17:05:24Z")\
**Posts on this page:** 9\
**Page:** 1

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 5:05pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/1 "2007-06-01T17:05:24Z")

</div>

Hey,

I just read this post:  
[http://www.therailsway.com/2007/6/1/railsconf-recap-skinny-controllers](http://www.therailsway.com/2007/6/1/railsconf-recap-skinny-controllers)

And it made me think immediately of the old handle\_new\_or\_edit\_post in  
page\_controller  
[http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/controllers/admin/page\_controller.rb?rev=57](http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/controllers/admin/page_controller.rb?rev=57)

But now I go and look at the latest version, and I see it’s been  
cleaned up a bit, and there a new AbstractModelController seperating  
the logic a bit.

[http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/controllers/admin/page\_controller.rb](http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/controllers/admin/page_controller.rb)  
and  
[http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/controllers/admin/abstract\_model\_controller.rb](http://dev.radiantcms.org/radiant/browser/trunk/radiant/app/controllers/admin/abstract_model_controller.rb)

But still… you have a lot of logic in the controllers. And a lot of  
that logic, if moved to the model, would be a lot easier to override  
and modify in extensions.

For instance, I’d like to see all the part-creating/arranging logic  
moved into the Page model.

96 parts\_to\_update = {}  
97 (params[:part]||{}).each {|k,v| parts\_to\_update[v[:name]] = v  
}  
98  
99 parts\_to\_remove = []  
100 @page.parts.each do |part|  
101 if(attrs = parts\_to\_update.delete(part.name))  
102 part.attributes = part.attributes.merge(attrs)  
103 else  
104 parts\_to\_remove \<\< part  
105 end  
106 end  
107 parts\_to\_update.values.each do |attrs|  
108 @page.parts.build(attrs)  
109 end  
110 if result = @page.save  
111 new\_parts = @page.parts - parts\_to\_remove  
112 new\_parts.each { |part| part.save }  
113 @page.parts = new\_parts  
114 end

So, my question to the core devs is… how open would you be to patch  
coming from me, that addresses some of these things.

I must admit, my agenda is based on making my own extensions more  
elegantly integrated with Radiant. But I’m hoping to appeal to any  
skinny-controller philosophy-followers as well.

thoughts?

Jacob

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 5:10pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/2 "2007-06-01T17:10:27Z")

</div>

We would love a patch, of course! This will also help in a future  
refactoring toward REST which is on the horizon.

Sean

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 5:32pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/3 "2007-06-01T17:32:30Z")

</div>

Thanks Sean.

Looks like posting on the mailing list gets a much faster reply that  
posting on Trac.

So then a follow-up question would be, what’s wrong with these  
patches, why havn’t they been replied-to?  
[http://dev.radiantcms.org/radiant/ticket/504](http://dev.radiantcms.org/radiant/ticket/504)  
[http://dev.radiantcms.org/radiant/ticket/502](http://dev.radiantcms.org/radiant/ticket/502)

Jacob

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 5:59pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/4 "2007-06-01T17:59:34Z")

</div>

I don’t know whether John has gone out of town yet or not, so I’ll do my  
best to reply in his stead.

In general, the mailing list is faster because it’s more in-your-face.  
Too, I think it’s a good principle to discuss something thoroughly on  
the mailing list, then create a patch or ticket on Trac that addresses  
the issue.

That said, I don’t often apply patches submitted via Trac; I let John  
make the decision. However, as a general rule, patches that provide  
small, incremental, well-tested changes that don’t break or  
fundamentally alter the functionality of Radiant are more likely to be  
accepted. Regarding #502, I think it’s a great idea, one I’ve been  
thinking about myself, but I have not tested it yet (for various  
reasons), and it changes a lot of core functionality. For #504, the  
\<r:navigation\> tag has been a sore spot for a long time and is unlikely  
to change in an incremental fashion. Also, just like the Rails core, if  
we don’t it useful that way, it’s just not likely to make it in. #504  
would be ripe for an extension, though – one of the reasons extensions  
exist, in fact.

If you look at the roadmap, there are also tickets on there that have  
existed for nearly a year – things that are important but have not  
found a solution that is satisfactory to the core team. We’re only  
human, and have a limited attention span and limited time to work on  
Radiant. We appreciate contributions partly because they save us time,  
and partly because they come from people who are passionate about the  
feature they want to implement or bug they want to fix. Sometimes the  
end result is not always exactly what was submitted, but open source is  
about the communication and participation.

Moral of the story – don’t be discouraged if your patch or ticket gets  
ignored. Discuss it on the mailing list and we’ll work it out together.

Cheers,  
Sean

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 7:46pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/5 "2007-06-01T19:46:46Z")

</div>

This is actually pretty old and the big thing missing from it is  
backwards-compatibility. One thing I personally would like to maintain  
about the navigation tag is that it is independent of the site  
structure. Anyway, here’s the pastie of my original idea:

[http://pastie.caboo.se/26340](http://pastie.caboo.se/26340)

Sean

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 7:24pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/6 "2007-06-01T19:24:42Z")

</div>

Jacob,

Your patch adding the level tag in \<r:navigation\> is quiet tidy. In  
my own Swiss Army tag library extension which I dump into nearly  
every Radiant site I put up I have a similar tag \<r:navigation\_level  
level=“2”\>. Your version is cleaner. My tag gets use in combined with  
an \<r:level /\> tag which returns the current depth/level in the tree.

Speaking to the bigger picture of core changes to the \<r:navigation\>  
tag I think that Sean is right to suggest a more drastic refactoring  
of the tag is in order. If I understand right Sean’s been playing  
with some of these more drastic revisions himself.

Sean, do I have you wrong? Can we have a peek?

Loren

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 8:42pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/7 "2007-06-01T20:42:54Z")

</div>

Sean C. wrote:

> One thing I personally would like to maintain  
> about the navigation tag is that it is independent of the site  
> structure.

Agreed. I’m not entirely sure I like the syntax of the new navigation  
tag. I wonder if there is a way to simplify it:

\<r:navigation\>  
\<r:items\>  
Web Log: /weblog/  
External Link: [http://externalsite.com/](http://externalsite.com/)  
\<r:children:each\>  
\<r:title /\>: \<r:url /\>  
\</r:children:each\>  
\</r:items\>  
\<r:normal\>\<r:title /\>\</r:normal\>  
\<r:here\>\<r:title /\>\</r:normal\>  
\<r:selected\>  
\<r:title /\>\</r:selected\>  
\<r:between\> | \</r:between\>  
\</r:navigation\>

Just a thought.

–  
John L.  
[http://wiseheartdesing.com](http://wiseheartdesing.com)

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 8:28pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/8 "2007-06-01T20:28:52Z")

</div>

Sean C. wrote:

> In general, the mailing list is faster because it’s more in-your-face.  
> Too, I think it’s a good principle to discuss something thoroughly on  
> the mailing list, then create a patch or ticket on Trac that addresses  
> the issue.

In our defense I’ve asked that people talk about changes and problems on  
the mailing list _before_ submitting a patch or ticket:

[http://dev.radiantcms.org/radiant/wiki#Contributions](http://dev.radiantcms.org/radiant/wiki#Contributions)

That being said, we’ve done a poor job at keeping up with the tickets on  
Trac and need to do a better at it.

> That said, I don’t often apply patches submitted via Trac; I let John  
> make the decision.

Sean, feel free to apply patches on Trac if you feel they are well  
tested and address the core goals of Radiant.

> Moral of the story – don’t be discouraged if your patch or ticket gets  
> ignored. Discuss it on the mailing list and we’ll work it out together.

Yup, yup. Good stuff.

–  
John L.  
[http://wiseheartdesign.com](http://wiseheartdesign.com)

---

<div class="post-metadata">

**Author:** ![igotimac](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/i/85f322/32.png) [@igotimac](https://www.ruby-forum.com/u/igotimac)\
**Post date:** [June 1, 2007, 8:47pm UTC](https://www.ruby-forum.com/t/radiant-core-dev-page-controller/105711/9 "2007-06-01T20:47:54Z")

</div>

> \</r:items\>  
> \<r:normal\>\<r:title /\>\</r:normal\>  
> \<r:here\>\<r:title /\>\</r:normal\>  
> \<r:selected\>  
> \<r:title /\>\</r:selected\>  
> \<r:between\> | \</r:between\>  
> \</r:navigation\>
> 
> Just a thought.

Much better!

Sean