[
http://jira.magnolia-cms.com/browse/MSHOP-46?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jan Haderka reopened MSHOP-46:
------------------------------
- {{ProductTeaserModel}} in case of {{RepositoryException}} you will wrap null
which will end up w/ NPE sooner or later
{code}
Node product = null;
90 if (StringUtils.isNotBlank(productUUID)) {
91 try {
92 product = NodeUtil.getNodeByIdentifier("data",
productUUID);
93 } catch (RepositoryException e) {
94 log.error("Can't find Product with UUID "+productUUID);
95 }
96 }
97 return new I18nNodeWrapper(product);
98 }
{code}
- exception handling in {{ShoppingCartItem}} is wrong (in more then just one
place quoted below).
{code}
} catch (RepositoryException e1) {
141 e1.printStackTrace();
142 }
{code}
- {{ShoppingCartItem}} - you can't change method signature for public class in
minor version like this. Actually more in general many of the API changes
introduced in this commit like generics require to make this major rather then
minor release.
{code}
182 public void setProductPrice(Content productPrice) {
public void setProductPrice(Node productPrice) throws ValueFormatException,
RepositoryException {
{code}
- exception handling in {{ShoppingCartParagraphModel}} is non existent.
{code}
} catch (Exception e) {
99 // TODO
100 }
{code}
- what are dependencies on asm and cglib necessary for? Since commit comment
doesn't explain it, there should be comment in the pom itself explaining what
it is for and in the issue as well.
- javadoc for {{ShopSingletonParagraphModel}} seems to be wrong. It claims that
model is control???
{code}
* shoppingcart control.
66 * @author tmiyar
67 *
68 */
69 public class ShopSingletonParagraphTemplateModel extends
STKPageModel<STKPage> {
{code}
- {{ShopTagCloudParagraph}} - search syntax doesn't look like jcr-sql2 and will
most likely fail or return nothing.
{code} templatingFunctions.search("data", "select * from category",
"JCR_SQL2", "")
{code}
- {{ShopProductSearchResultParagraphModel}} and
{{ShopKeywordSearchResultParagraphModel}} exception handling is not correct.
{code}
} catch (Exception e) {
72 //TODO
73 }
{code}
- also keeping around commented out code is unnecessary
{code}
/*protected String generateSimpleQuery(String searchString) {
79 //escape single quote
80 searchString = searchString.replace("'", "''");
81 return MessageFormat.format(SEARCH_QUERY_PATTERN, new
String[]{ShopUtil.getShopName(), searchString});
82 }*/
{code}
- {{productList.ftl}} before change {{pager}} was not mandatory, now it is. Are
you sure this is correct change? It should be explained in the commit comment
or at the very least in the issue itself.
- {{DefaultShoppingCart}} exception handling is not correct
{code}
} catch (RepositoryException e) {
136 e.printStackTrace();
137 }
{code}
- {{ShopParagraphModel}} exception handling is not correct (in more then just
one place quoted below).
{code}
} catch (RepositoryException e) {
156 // TODO Auto-generated catch block
157 e.printStackTrace();
158 }
{code}
General comments
- the fact that this issue links to many others and combines all of those
changes in one commit makes proper review very complicated and cumbersome. It
is not possible to review and assert each of the changes individually
- many of the changes here - API changes require major version release.
- many of the java code changes require tests
- exception handling of the whole module seems to be nearly non existent and
needs to be improved.
> Migrate code to the latest Magnolia API
> ---------------------------------------
>
> Key: MSHOP-46
> URL: http://jira.magnolia-cms.com/browse/MSHOP-46
> Project: Magnolia Shop
> Issue Type: Improvement
> Affects Versions: 1.1
> Reporter: Teresa Miyar
> Assignee: Teresa Miyar
> Fix For: 1.1.1
>
>
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.magnolia-cms.com/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
----------------------------------------------------------------
For list details, see: http://www.magnolia-cms.com/community/mailing-lists.html
Alternatively, use our forums: http://forum.magnolia-cms.com/
To unsubscribe, E-mail to: <dev-list-unsubscr...@magnolia-cms.com>
----------------------------------------------------------------
