[#XWIKI-2882] Implement RSS macro for new rendering

XWiki Core

Implement RSS macro for new rendering

Created: 26/Nov/08 14:21 Updated: 18/Mar/09 22:01

Component/s:

Rendering 2.0

Affects Version/s:

1.7 M3

Fix Version/s:

1.8 RC1

File Attachments:

XWIKI-2882-1.patch (34 kB)
2.

XWIKI-2882-2.patch (38 kB)
3.

XWIKI-2882-3.patch (35 kB)
4.

XWIKI-2882-4.patch (27 kB)

Issue Links:

Dependency

This issue blocks:

XWIKI-3383 Make the Syntax converter able to convert RSS Radeox macro to the new rendering RSS Macro

This issue depends on:

~~XWIKI-3197~~ Add a set/getBlockTitle in BoxMacroParameters based > on List<Block> which can be used from java

~~XSTOUCAN-71~~ Several Css fixes in order for the Rss and Message macros to display properly

~~XWIKI-2967~~ Add ability to display an optional image and a optional title in the Box macro

~~XSALBATROSS-51~~ Several Css fixes in order for the Rss and Message macros to display properly

Date of First Response:	17/Jan/09 19:13
Resolution Date:	12/Feb/09 15:40
Tests:	Unit

Description

« Hide

See http://code.xwiki.org/xwiki/bin/view/Macros/RssMacro

All

Comments

Change History

FishEye

Sort Order:

[ Permlink | « Hide ]

Dan M added a comment - 17/Jan/09 19:13

This one implements the macro and it also includes a test case for it.

[ Permlink | « Hide ]

Thomas Mortagne added a comment - 28/Jan/09 15:33 - edited

this macro should not depends on radeox, expecially if it's just to use org.radeox.util.Encoder. You should directly generate the List<Block> and not generate a html string and call html macro to do it in RssMacro.renderSearch.

in RssMacro.renderEntries() you should use ParserUtils.parseInlineNonWiki to parse the description, WordBlock is supposed to contains only one work and not an entire paragraph.

never use wiki syntax in macros
- macro is supposed to not be syntax dependent
- a macro is supposed to work on XDOM level
- create directly the right block is way faster
  If the problem is that you can't pass List<Block> for the title to BoxMacroParameters, do it another way:
- you could define a protected List<Box> getTitle() methods in AbstractBoxMacro like you have getClassProperty() and extend it with a RSSEntryMacro class
- you could also add a set/getBlockTitle in BoxMacroParameters based on List<Block> which can be used from java (we could even imagine to be able to use it from wiki syntax with the tight BeanUtil converter)
- ...

[ Permlink | « Hide ]

Dan M added a comment - 31/Jan/09 21:27

It fixes the issues reported by Thomas:
-removes the radeox dependency
-no longer invokes the html macro
-no longer uses WordBlocks
-it generates the titles without using wiki syntax.

[ Permlink | « Hide ]

Thomas Mortagne added a comment - 02/Feb/09 16:58

Some more issues:

You should not have the "<" and ">" for INPUT and DIV since or it will generate <<div>> in place of <div>
You should have more unit tests, you don't test all the feature of this macro (especially with a unit test on the renderSearch you would not have the first issue I guess)
Isn't rss macro result supposed to be in a box ? I don't see any <div class="box"> or something similar in the expect|xhtml/1.0 of the unit test.

[ Permlink | « Hide ]

Dan M added a comment - 04/Feb/09 16:13

This has a cleaner test case. On the previous patch, it was using a mock instead of the real boxMacro, that's the reason why the output looked so strange, now it's fixed.

[ Permlink | « Hide ]

Jerome Velociter added a comment - 12/Feb/09 15:17

Latest patch from Dan

[ Permlink | « Hide ]

Jerome Velociter added a comment - 12/Feb/09 15:40

Patch applied with modifications, see r16661 commit log
Now creating issues to adpat a bit albatross and toucan css in their own projects

[ Permlink | « Hide ]

Vincent Massol added a comment - 12/Feb/09 15:51

I think there are some tests missing that could be good to add:

tests of all parameters
test of failure conditions (what happens if the remote site doesn't answer)