[GitHub] [ofbiz-framework] PierreSmits opened a new pull request #184: Fixed: Overview of shipments shows wrong link to shipment (OFBIZ-11787)

[GitBox]

PierreSmits opened a new pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184


   See commit message.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

sonarcloud[bot] commented on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637344892


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=VULNERABILITY) (and [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=SECURITY_HOTSPOT) to review)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_ofbiz-framework&pullRequest=184&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_ofbiz-framework&pullRequest=184) No Coverage information  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_ofbiz-framework&pullRequest=184&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_ofbiz-framework&pullRequest=184&metric=new_duplicated_lines_density&view=list)
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

verma-pawan commented on a change in pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#discussion_r433679729



##########
File path: applications/product/widget/facility/ShipmentForms.xml
##########
@@ -70,12 +70,12 @@ under the License.
                 <field-map field-name="viewSize" from-field="viewSize"/>
             </service>
         </actions>
-        <field name="shipmentId" title="${uiLabelMap.ProductShipmentId}" widget-style="buttontext">
-            <hyperlink description="${shipmentId}" target="EditShipment">
+        <field name="shipmentId" title="${uiLabelMap.CommonShipment}" widget-style="buttontext">

Review comment:
       Hi @PierreSmits
   I think we can use CommonShipmentID here, WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

PierreSmits commented on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637362713


   Context is everything. And a Shipment is represented with its Id. So why not keep it simple and lose the Id in the label when the obvious is obvious?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

PierreSmits commented on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637363898


   I am confident, that many don't need the inclusion of Id in the label.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

PierreSmits edited a comment on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637363898


   I am confident, that many don't need the inclusion of Id in the label when the context is clear and unambiguous.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

PierreSmits edited a comment on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637363898


   I am confident, that many don't need the inclusion of Id in the label when the context is clear and unambiguous. Similarly like not including Id on status labels (uiLabelMap.*StatusId).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

verma-pawan commented on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637433783


   > Context is everything. And a Shipment is represented with its Id. So why not keep it simple and lose the Id in the label when the obvious is obvious?
   
   Yes, I agree, Context is everything. But we need to maintain consistency throughout the system. We can change this pattern all over the application in a separate effort.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

PierreSmits commented on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637439216


   Indeed. And I have suggested such changes regarding simplification of labels (and application thereof) a few times in the past, but that was met with too much obstruction from some contributors. Mainly being about patches being either to small or to big. And that is a pity.
   
   Consistency (and ease of use) should be key, but is nevertheless a big endeavour that will take effort. See my attempts to get to a consistent way of having a Main Action Menu (for the creation process of the most important objects) in each web app. (see other PRs).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

PierreSmits edited a comment on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637439216


   Indeed. And I have suggested such changes regarding simplification of labels (and application thereof) a few times in the past, but that was met with too much obstruction from some contributors. Mainly being about patches being either to small or to big. And that is a pity.
   
   Consistency (and ease of use) should be key, but is nevertheless a big endeavour that will take effort to implement. See my attempts to get to a consistent way of having a Main Action Menu (for the creation process of the most important objects) in each web app. (see other PRs).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

verma-pawan closed pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

verma-pawan commented on pull request #184:
URL: https://github.com/apache/ofbiz-framework/pull/184#issuecomment-637487149


   I've merged changes manually so closing this PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]