Replaces #1702 #1706
Replaces #1702 #1706
Conversation
|
@joemsak I'd love your feedback on this and to know whether it fixes your problem too |
…all super on rebuild!
|
|
||
| def rebuild_with_invalidate_cached_urls! | ||
| rebuild_without_invalidate_cached_urls! | ||
| find_each.each(&:invalidate_cached_urls) |
There was a problem hiding this comment.
I think the each call is redundant here.
There was a problem hiding this comment.
Can someone explain when this gets called during update_positions in crud (calls class method rebuild! -- what is alias_method_chain doing here?)
and then what this rebuild_with/without method is doing? I'm a bit mystified by it.
I understand why my earlier method didn't need to iterate over children...
There was a problem hiding this comment.
alias_method_chain is a way of overriding a method but retaining the original version.
In this case alias_method_chain creates a method called rebuild_with_invalidate_cached_urls! and rebuild_without_invalidate_cached_urls!
The 'without' method is the original and the 'with' method is the new one that we've just defined. The first step is calling the the original and then we're calling our custom logic. This ensures that awesome_nested_set can still run its logic.
However, I can put a raise in this method and nothing fails suggesting that we're not testing that this method gets called at all. Fail.
There was a problem hiding this comment.
I forgot to mention that under the hood rebuild! is now mapped (using alias_method to rebuild_with_invalidate_cached_urls! so calling Refinery::Page.rebuild! executes our new method instead.
There was a problem hiding this comment.
@phiggins you were quite correct; I have updated that.
|
Aha okay I totally get about 90% of what you said so thats good enough for me +1 thanks for the hard work |
|
So this code solves your problem? |
|
Why is this needed in the first place? I fail to reproduce a failing case. @joemsak? |
|
Refinery 2.0.4: Open to /refinery/pages Re-sort a page to a new parent page The original problem was that at this point, the admin edit link was invalid and produced a nil @page object I "resolved" it by sticking to the primary key ID instead of the friendly id path in the edit URL constructor The second problem that arose from fixing the first was that our public side submenu would show the page in the right spot but with the wrong URL, producing a 404 when clicked Our submenu was using refinery.url_for(page.url) -- I'd have to dig to remember why but suffice to say we had to use that method to make page urls work both for pages with marketable urls (friendly id) and pages that have forwarding urls (link_url) When admin/pages#update_positions is called through the action of clicking "done resorting pages" it's easiest to hook into the line that calls class method "rebuild!" |
|
I see the re-sort bug in backend now - thanks @joemsak! Btw, it's still present. We need page reload or some kind of jquery stuff to change the url for edit link. |
|
Youd have to have update_positions render the partial after rebuilding the urls but it's a shared crud.rb action so I'm sure it's tricky |
|
@ugisozols happy? |
|
Yes. Are you? :) |
|
I generally have a positive outlook on life, yes. Also I'm happy with this pull request but then it's mine.. |
No description provided.