Workbench for browsing categories and products (MGNLWCS-6)

[MGNLWCS-50] Workbench for browsing categories and products Created: 02/Sep/14  Updated: 15/Sep/14  Resolved: 02/Sep/14

Status: Closed
Project: Websphere Commerce Integration
Component/s: None
Affects Version/s: None
Fix Version/s: 1.0

Type: Sub-task Priority: Neutral
Reporter: Jaroslav Simak Assignee: Robert Šiška
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relation
is related to MGNLWCS-47 Asset provider should also work with ... Closed
Template:
Date of First Response:

 Comments   
Comment by Robert Šiška [ 02/Sep/14 ]

Commits:

https://git.magnolia-cms.com/gitweb/?p=enterprise/websphere-commerce.git;a=commit;h=9ce6e229332b622227ff97bea8d00e0733773ff8
https://git.magnolia-cms.com/gitweb/?p=enterprise/websphere-commerce.git;a=commit;h=e728b2eb1895f3280bd96eb51c251a3c0d3f499c
https://git.magnolia-cms.com/gitweb/?p=enterprise/websphere-commerce.git;a=commit;h=7bb1988b758b96376965c4293621e99207492e83
https://git.magnolia-cms.com/gitweb/?p=enterprise/websphere-commerce.git;a=commit;h=fdb95ea39182f69301537f0e9fae4b5cf16b710d

Comment by Jan Haderka [ 05/Sep/14 ]
  • Review comments everywhere. Many of /TODOs are actually "not used at the moment".
  • For each real /TODO create a jira ticket and mention it in the code.
    There are also comments like "proof of concept ..." and similar. Those should go.
  • Review exception handing. Just e.printStackTrace() is not good enough.
  • In CategoryViewImpl and ProductViewImpl there's lots of code, specially in html part that seems to be originated in other module. Clean it up. Use only what you need, name it correctly, get rid of the rest.
  • Number of classes are marked as "unused". Delete them. They stay in git in case you ever need to recover them.

(some of those were already fixed by later commits, but since there's no comprehensive overview of code changes, pls double check)

In general lot of that code looks very generic and should be extracted to abstract/common parent or back in DAM or in UI in general. While not necessary right now, keep it in mind and preferably contribute it back to DAM or you will have to reimplement it again, once DAM or UI provides similar functionality.

Comment by Aleksandr Pchelintcev [ 05/Sep/14 ]
  • Why do you make CategoryView/ProductView and their presenters to inherit from ContentView/Presenter?
  • The SplitView/Presenter which aggregates them already inherits ContentView/Presenter, so the sub-views can pbbly be just View's.
  • CategoryViewImpl - I'd move content connector operations to presenter.

Otherwise, looks good and seems to go into a correct direction from App Framework point of view.

Comment by Robert Šiška [ 08/Sep/14 ]

Most of the issues pointed out by Jan are about code which is based on emerging asset app prototype.

When the DAM 2.1 is released, that code will be removed.

Generated at Mon Feb 12 11:12:47 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.