refactor inclusion processing
Ticket
+issues
inclusions have been processed as part of the "slot" handling, which adds special html wrappers cards so that their "card slots" can have special behavior in browsers. This is not desirable, because we often want to process inclusions in contexts where these slots are irrelevant (xml, email, etc).
+solution
We're moving inclusion processing to the Renderer, with callbacks to slot.rb as appropriate
We're seeing problems with item:link and item:name inclusions, so reports of other problems would be especially useful.
http://www.dwagn.org/card/edit/ADA_compliance_for_Images_alt_title_support gets an App Error for the whole page. Hoptoad says "NoMethodError: undefined method `fancy_title' for #". http://www.dwagn.org/wagn/ADA_compliance_for_Images_alt_title_support just shows an error rendering the _main card. ADA compliance for Images (alt & title support) is fine.
--John Abbe.....Tue Dec 14 13:29:48 -0800 2010
Looks like "naked" view isn't generating the correct content on searches -- it should actually run the search, not just show the WQL (naked is not raw). John Abbe pointed out the problem here: new maybe broken web address extras behavior
--Ethan McCutchen.....Sat Jan 01 17:06:44 -0800 2011
Just so I'm clear, this is a problem with just the new code, and John's example shows the issue?
Can we create a failing test case for it before fixing?
--Gerry Gleason.....Sun Jan 02 07:08:54 -0800 2011
Oh, I found the dwagn login info and can see it. I should be able to get to this today. I'm trying to finish up the big card_id refactor, it's really starting to converge now. I'm down to 43 bad tests, 100% of unit tests passing.
--Gerry Gleason.....Sun Jan 02 07:16:24 -0800 2011
Added the tests, the search | naked is still broken, but there is a failing test for it now.
These two commits on develop:
commit ccf99583050b4e50aeda0090e9f558ebce2c42c6
Author: Gerry Gleason
Date: Sun Jan 2 10:29:07 2011 -0600
Added test for titled too
commit 20579bf2895ccfc9ef7e3228bc4ef7d0cecb5511
Author: Gerry Gleason
Date: Sun Jan 2 10:17:25 2011 -0600
Added tests for link, name and search|naked, last one still failing
--Gerry Gleason.....Sun Jan 02 08:32:42 -0800 2011
I think I fixed the case you found, but noticed that 'item' views may not be respected as a further modifier. Can you confirm that the tests for this make sense?
New commit is: d185903901fb07edcae60c95821dab177135c172
--Gerry Gleason.....Sun Jan 02 10:27:19 -0800 2011
hmm, the tests don't look right to me. Here's what I'm seeing:
Slot.new(c).render( :naked ).should == %{}
81
+ d = Card.create :name => 'AsearchNaked1', :content => "{ {Asearch|naked;item:name} }"
82
+ Slot.new(c).render( :naked ).should == %{["Mr. Buttz", "", "I got no account", "I'm number two", "", "I'm number one", "", "", "", ""]}
83
+ Slot.new(d).render( :naked ).should == %{"Mr. Buttz"}
82 84
end
The idea of "naked" is that it should be the exact same as "content", but without the wrapper. So the default case of a search rendered in naked view will be that of several nested cards rendered in closed view. If the item is set to "name", then you should see all the names of the items in the collection, with the following wrapper:
It looks to me like the test samples are much more minimal.
--Ethan McCutchen.....Sun Jan 02 13:32:31 -0800 2011
The test cases aren't very good, I could use some help with those whether it works now or not. I think the values are "correct" if I just string them together without the "s and []'s, but the item views don't work. Maybe we just fill in what a working version of Wagn outputs for the cases we have.
--Gerry Gleason.....Sun Jan 02 13:53:24 -0800 2011
Much improved tests are in this commit 3ca72a8dca9aec96f2dfcb1f678de17aa1c2d62d
The case that is failing is, I think, the one John's ticket is about, and I think it may be broken in master. I had a little trouble backporting the tests, but I think I'm now seeing the same failure mode in my code and master.
It looks like it may be a bug in deployed Wagn after all. Can we verify that?
--Gerry Gleason.....Mon Jan 03 08:31:50 -0800 2011
But wagn.org is running master, and John's support ticket points to the correct behavior on wagn.org and the incorrect behavior on dwagn.org (or some subdomain), which is running on develop. I think this is a new bug. Will look at test. Interesting about the pasted code breaking things. do we not scrape comments the way we scrape other html? sure ought to!
--Ethan McCutchen.....Mon Jan 03 09:26:49 -0800 2011
We don't scrape for transclusion syntax, because you can legally enter transclusion code directly, and the editors don't really have support for that syntax in any case, so that's the only way it comes in. What is odd is how it effected the page.
You're probably right about where the bug is coming from, but I think it is worth looking at what master does with these tests. I did try that, but without complete success. That's part of how I figured out what current code thinks the output should look like. Now I'm not sure if what I put it the test it right, but it should work with master. The bug I found may be present in master, but not the test case from John's ticket.
It would help a lot if you made sure we have some good tests for this now, and whether it is still failing with the latest change.
In other words, I think the transclusion should have rendered as a transclusion, probably a missing card since the card reference is from a test.
--Gerry Gleason.....Mon Jan 03 10:35:52 -0800 2011
What kinds of things should I test?
--John Abbe.....Sat Jan 15 18:16:07 -0800 2011
Generally, search results in various contexts have been one of the sticky points. Item views in pointers and searches both have had some problems that our automatics tests have missed.
--Ethan McCutchen.....Sat Jan 15 18:46:04 -0800 2011
Related tab wonkiness - on http://sandbox.dwagn.org/card/related/John_Abbe the "Administrator" link goes to http://sandbox.dwagn.org/wagn/known-card (same with the links to every editor under "community"). Also, items of the included Search on "incoming" which should be link view are a weird sort of open/closed.
(Posted originally on CSS on dwagn not being respected, dunno if the main problem there relates to this or not.)
--John Abbe.....Thu Jan 20 02:12:56 -0800 2011
Similarly, try add/editing the tags on http://www.dwagn.org/wagn/custom_hover_text and then cancel (or save) and you get the same weird open/close view.
--John Abbe.....Thu Jan 20 16:13:45 -0800 2011
The user links to "known-card" are no longer happening, but the weird open/closed view still is. And when you click on the name of one of those, that card suddenly fills the card slot.
I think Ethan has seen some odd behavior as well...
--John Abbe.....Sun Jan 23 03:47:44 -0800 2011
note to self: when the weird view thing is solved, re-check to see if process markup on items in forms that are Pointers is still relevant.
--John Abbe.....Thu Jan 20 20:12:35 -0800 2011
I'll try to test all the custom cardtype views, the problems mentioned above, and additional banging on this on Thursday. The next time I'd have a chance after that will be Monday...
--John Abbe.....Tue Jan 25 15:03:51 -0800 2011
are the todos in +todo todone?
--Ethan McCutchen.....Tue Jan 25 20:06:20 -0800 2011
Above open/closed view problem no longer appearing.
1. Nonexistent inclusions are showing the root card as part of card names even when the inclusion is contextual, e.g. http://test.dwagn.org/wagn/inclusion_refactoring_tests shows "Add inclusion refactoring tests+contextual name doesn't exist" when what's included on the card is "+contextual name doesn't exist"
2. Editing then saving (or cancelling) Pointers included with titled view adds the title again. Again, see http://test.dwagn.org/wagn/inclusion_refactoring_tests
--John Abbe.....Tue Jan 25 20:11:18 -0800 2011
is #2 definitely new?
--Ethan McCutchen.....Tue Jan 25 21:12:46 -0800 2011
I've never seen it before.
--John Abbe.....Tue Jan 25 21:30:56 -0800 2011
3. titled is broken with "unknown card view: 'tiitled' M:nil" - presumably a typo in the code. See http://test.dwagn.org/wagn/inclusion_refactoring_tests
--John Abbe.....Tue Jan 25 21:41:36 -0800 2011
Maybe unconnected to this ticket, but http://test.dwagn.org/account/signup gets NoMethodError: undefined method `chk' for #
--John Abbe.....Tue Jan 25 21:59:49 -0800 2011
Ethan must have fixed a few of these just now. I don't see tiitled anywhere in the code, and signup seems to work as well on the latest code. This code maybe isn't on dwagn yet, but in the changes I just pulled from wagn/develop
--Gerry Gleason.....Wed Jan 26 04:31:12 -0800 2011
Frustrating if I'm wasting my time. Will keep reporting for now, but if there are other changes happening soon, maybe we should set this back to "in progress" ?
4. Seems likely to be this ticket: http://test.dwagn.org/Default_Layout has
[[Ticket]]
--John Abbe.....Wed Jan 26 07:17:56 -0800 2011
Maybe not related: when I edit http://test.dwagn.org/Default_Layout then click on View, I get NoMethodError: undefined method `name' for nil:NilClass
(If these potentials are related, I'll number them, otherwise I'll move them to an appropriate ticket.)
--John Abbe.....Wed Jan 26 07:28:11 -0800 2011
We're on a pretty fast cycle now, so what you're seeing is either the latest and greatest or pretty damn close.
The MAIN thing is intentional -- had to do something to keep _main from recursing. see above.
I moved stuff into todos. Let's keep incrementing the numbers even if we solve and wipe old ones so the discussion is consistent.
Thanks guys!
--Ethan McCutchen.....Wed Jan 26 09:20:52 -0800 2011
Not sure if I have seen #2 from todo, but it seems ok to me now.
I also think I have a fix for #3 (posted to gerryg/slot_render).
Also, just posted a test for #1, but haven't fixed it yet.
--Gerry Gleason.....Wed Jan 26 12:54:04 -0800 2011
On #6, I have a little more information. It isn't getting a "main_card" in this case, and I'm not sure that is wrong. I have a fix that renders this case as "NO MAIN", but maybe this is just the same as the recursive render case, only we didn't come in at the top. That is, we are rendering a that isn't at the top level, but it is under an xhr request now, and main_card isn't set on the "root" of the render.
Confused yet?
--Gerry Gleason.....Wed Jan 26 13:10:55 -0800 2011
#3 was two issue, I think. The view was misspelled in the test card. Also it was tripping the same bug as #2 and would give _tiitled_missing or even _titled_missing if you spelled it right.
--Gerry Gleason.....Wed Jan 26 14:48:43 -0800 2011
On #4, I improved it and fixed the test. now it should give the original inclusion { {_main|naked} } in the test.
--Gerry Gleason.....Thu Jan 27 03:07:14 -0800 2011
10. http://test.dwagn.org/wagn/hee_hee - It's a cardtype card, and the settings subtab shows closed view cards for every cardtype setting (below the card settings), even those that don't exist.
--John Abbe.....Thu Jan 27 23:03:55 -0800 2011
thanks john. fixed that. I had recently broken it with another fix. related problem had busted Recent Changes.
--Ethan McCutchen.....Fri Jan 28 11:17:16 -0800 2011
Checked off all the existing issues 1-10, except #2.
--John Abbe.....Fri Jan 28 14:02:22 -0800 2011
K, fixed #2. Given how long it's annoyed me that the title has always been outside of the wrapper -- which means it was still there when you edit -- it's crazy how easy that was to fix. Probably wouldn't have been as easy if I were loaded up on the slot stuff. Or if it weren't as clean as it now is...
Doing so addressed the problem in #2 (which, fwiw, I highly doubt was specific to pointers).
Anyway, that made me happy :)
--Ethan McCutchen.....Fri Jan 28 16:05:23 -0800 2011
2 checked out.
--John Abbe.....Fri Jan 28 23:25:28 -0800 2011
From Wagn 1.05+2's discussion:
- √(testing) html not getting "cleaned" (saves with unwanted styling)
strikethrough and a hand-created style attribute setting font to Times got removed from http://test.dwagn.org/wagn/inclusion_refactoring_tests - does this need deeper testing? --John (No.) - √(testing) settings values are not opening in place (they take up the whole row)
not seeing anything weird now adding or editing settings. --John - √(testing) layouts are rendering recursively
http://test.dwagn.org/wagn/Default_Layout looks ok to me; closing. --John - √(testing) inclusions are being rendered on form cards (they shouldn't)
- √(testing) searches screwed up in closed view
included all star cards on http://test.dwagn.org/wagn/inclusion_refactoring_tests looks ok. --John - √(testing) displaying long rendering errors inline
--John, 1/31/11, 2/11/11
Great work on the testing. I can shed light on a few of these.
Recursive layouts: when viewing Default Layout, it does not keep including 'main' again and again. Now, it doesn't expand main below the first render level. There is a test for then and it is passing.
Inline errors -- It used to just say 'error rendering' when a bug or other problem throws on error, and I changed it to put in the details, but it was supposed to be just in debugging mode. Ethan moved the detail into the mouse-over text. You would have to create rendering error to test it, but I'm not sure of a way to do that reliably. I could add a bug for you ;)
--Gerry Gleason.....Mon Jan 31 09:27:59 -0800 2011
Thanks. --John
Also from Wagn 1.05+2's discussion:
- main_content / main_card correctly handled layout
If this is just about _main in layouts I think it's all good.
- main card-slot shows up with correct attributes
By "attributes" you mean this? :
div id="main" context="main"><div base="self" cardId="2264" class="card-slot paragraph ALL TYPE-basic SELF-inclusion_refactoring_test" position="a14c7c9" style="" view="open"
- javascript added when needed
I've edited and added things a variety of ways on http://test.dwagn.org/wagn/inclusion_refactoring_tests and it seems okay. Not sure what else to test.
- correct css classes on wrapped slots
Looks good to me, e.g. on http://test.dwagn.org/wagn/inclusion_refactoring_tests :
class="card-slot line ALL TYPE-basic RIGHT-contextual_name_exist TYPE_PLUS_RIGHT-basic-contextual_name_exist SELF-inclusion_refactoring_test-contextual_name_exist"
- correct requested view in content wrapper
seeing:
view="content"
...in the div tag for "finding empty pointer" included as content view on http://test.dwagn.org/wagn/inclusion_refactoring_tests
- slot_wrap helper method working
- correct handling of special case inclusions (content cards, _main in layout cards)
Unsure how/what to test on these last two.
Whup, now I get this list was about automated tests. Delete this?
--John, 1/31/11
Still helpful (even though it's targeted at automation). Looking at the above and the bulleted list, I'm not seeing anything *specific* that obviously needs more testing than you've done. Given what you've written, it looks like most of the known problems have been addressed.
So I guess at this point we're sort of hunting for weirdness, a practice that I hope we get to do less and less of as our testing suite improves.
Thanks John!
--Ethan McCutchen.....Mon Jan 31 09:56:32 -0800 2011
I haven't gone through custom cardtype views systematically yet, good chance I'll get to it tonight. But at this point most if not all of it has probably been seen by somebody.
--John Abbe.....Mon Jan 31 17:48:44 -0800 2011
Did a little, will try to finish tomw but I'm comfortable with us going ahead if not. Anything remaining is probably edge-y.
11. On http://test.dwagn.org/wagn/testing_Pointer_inputs the main card's footer is detached from the card. That is, the white part has rounded corners, and the footer appears a few pixels below. When I close the main card the detached footer remain, and when I re-open I see two footers.
--John Abbe.....Mon Jan 31 20:40:10 -0800 2011
Can we turn on Script cards on test so that I can test views for them?
--John Abbe.....Mon Jan 31 20:48:24 -0800 2011
The template (Committee+*type+*content) had a bunch of nested p's and spans that clearly confused firefox (though, somehow, not Chrome). I could do another old deploy if we really want to test this, but I strongly suspect that the old code would have rendered it the same way and evinced the same weirdness.
--Ethan McCutchen.....Mon Jan 31 20:57:01 -0800 2011
Headers *inside* of closed cards are being picked up by the ToC, which seems wrong to me. If we don't want to hold things up for that, I can ticket it.
--John Abbe.....Tue Feb 01 06:20:37 -0800 2011
We have convert p to div. Could we ticket something like remove attribute-less spans? Both when saving and as a migration I'm thinking. I run into that sort of thing fairly often.
--John Abbe.....Tue Feb 01 06:29:28 -0800 2011
Did a decent run through custom cardtype views except for Script, and I think we're good.
--John Abbe.....Tue Feb 01 07:06:01 -0800 2011
Re ToC, yes let's ticket that for now -- it's not easy or unobtrusive enough to stick in. I think we may want to give some more thought to how we want the ToC to work now. For example, do we want to treat an h1 inside an included card as a level deeper in the ToC outline or back to the top level as we do now?
Re attribute-less spans: are you seeing associated problems? Even an attributeless span can be significant to CSS, so we may cause some problems by removing them. But worth exploring.
will try to remember to turn on script cards later today, but I'm not entirely sure how we'd test them. Don't want to make a big project out of it, but will do if easy.
--Ethan McCutchen.....Tue Feb 01 09:26:33 -0800 2011
ignore headers inside of closed inclusions when building ToC
re treatment of h1s inside inclusions, in cases where I've been aware of it, the actual tag reflects what would look best - e.g., I've used h2s when that's what I've wanted. In any case, I think it's something we'll actually get to see now and can learn from reality.
I'm pretty sure I've seen messes created by a bunch of nested attribute-less spans. When you say we could cause problems, you mean there's CSS thet depends on nested spans that have been inserted into cards' HTML? I'm not talking about removing any spans from outside of card content.
Re scripts yeah, seems unlikely to be a source of problems, but seems like a good idea for completeness' sake on the inclusion refactor, and just generally a good idea to have them available on our test wagn.
--John Abbe.....Tue Feb 01 13:47:25 -0800 2011
per an offline conversation, have ToCs reflect only some headers in inclusions
--John Abbe.....Mon Feb 07 14:11:00 -0800 2011
I think this ticket may be ready to retire. Do you feel like there's anything still open here that isn't represented elsewhere that we should address before closing?
--Ethan McCutchen.....Mon Feb 07 18:13:20 -0800 2011
Minor, but just waiting for Script cards to be turned on.
--John Abbe.....Mon Feb 07 18:52:00 -0800 2011
Script/Ruby problem found and ticketed.
--John Abbe.....Fri Feb 11 11:27:20 -0800 2011