[
https://issues.apache.org/jira/browse/OFBIZ-2436?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12707623#action_12707623
]
Vikas Mayur commented on OFBIZ-2436:
------------------------------------
Hi Akash,
I would suggest few changes:
1)
{quote}
+ boolean enteredProdIsOrdProd = false;
+ List<GenericValue> orderItems =
delegator.findByAnd("OrderItem", UtilMisc.toMap("orderId", orderId));
+ for (GenericValue orderItem : orderItems) {
+ if (productId.equals(orderItem.getString("productId"))) {
+
orderItemSeqIds.add(orderItem.getString("orderItemSeqId"));
+ enteredProdIsOrdProd = true;
+ }
+ }
+ if (enteredProdIsOrdProd) {
+ for (String orderItemSeqId : orderItemSeqIds) {
+ counter++;
{quote}
This can be simplified using something like
{quote}
List<GenericValue> orderItems = delegator.findByAnd("OrderItem",
UtilMisc.toMap("orderId", orderId, "productId", productId));
if (UtilValidate.isNotEmpty(orderItems)) {
for (GenericValue orderItem : orderItems) {
// Rest of the code under if (enteredProdIsOrdProd) {, can be moved here
}
}
{quote}
I know that this code is just moved over one file to another and was certainly
missed during my first commit :)
2)
{quote}
+ <property
key="ProductErrorNoValidOrderItemFoundForProductWithEnteredQuantity">
+ <value xml:lang="en">ERROR: No valid order item found for product [
${productId} ] with quantity: ${quantity}</value>
+ </property>
{quote}
can be
{quote}
<value xml:lang="en">ERROR: No valid order item found for product
[${productId}] with quantity [${quantity}]</value>
{quote}
3) In this method
{quote}
+ public boolean isSameItem(VerifyPickSessionRow line) {
+ if (this.getInventoryItemId().equals(line.getInventoryItemId())) {
+ if (this.getOrderItemSeqId().equals(line.getOrderItemSeqId())) {
+ if (this.getOrderId().equals(line.getOrderId())) {
+ if
(this.getShipGroupSeqId().equals(line.getShipGroupSeqId())) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ }
{quote}
This can be simplified using
{quote}
if (condition1 && condition2 && condition3 && condition4) {
return true;
} else {
return false;
}
{quote}
4) It is always good to add brief comments to explain some snippets, like for
example in #1, the boolean variable is self explanatory but only to a certain
extent. It helps a lot to a person reviewing the code as well in maintaining
the code later on (and doing any subsequent changes in future) as with the
comment code would be self explanatory.
Rest looks good. Good Work!
Vikas
> Improvement in Verify Pick screen to issue items as well when shipment is
> created
> ---------------------------------------------------------------------------------
>
> Key: OFBIZ-2436
> URL: https://issues.apache.org/jira/browse/OFBIZ-2436
> Project: OFBiz
> Issue Type: Sub-task
> Components: product
> Affects Versions: SVN trunk
> Reporter: Pranay Pandey
> Assignee: Vikas Mayur
> Priority: Minor
> Fix For: SVN trunk
>
> Attachments: ItemIssuance.patch, ItemIssuance.patch
>
>
> Improvement in Verify Pick screen to issue items as well when shipment is
> created in Picked status:
> # When shipment is created and marked PICKED should also be issued items
> (ItemIssuance)
> # Once order items are verified on this screen, Packing screen should use the
> same shipment created.
> # Also reduce code for Packing which has to be moved to Very Pick now.
> *The process will be:*
> # Create a sales order with 4 to 5 items.
> # Go to Facility-->Verify Pick, enter orderId and very order items.
> # After all items are verified shipment should be created in PICKED status,
> invoice will be generated in INVOICE_IN_PROCESS along with item issuance.
> # Now go Facility-->Packing, enter verified oderId and complete pack, now
> this process will utilize the same shipment, invoice created in Verify Pick
> process only status of shipment will be changed to PACKED after completion.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
