Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OBPIH-5825 Add comments count on comments tab #4325

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

drodzewicz
Copy link
Collaborator

  • add comment count badge
  • sort comments desc by last updated
  • show comments only on requests (check by SourceType == Electronic)

- add comment count badge
- sort comments desc by last updated
- show comments only on requests (check by SourceType == Electronic)
@@ -12,7 +12,7 @@
</tr>
</thead>
<tbody>
<g:each var="comment" in="${requisition?.comments}">
<g:each var="comment" in="${requisition?.comments?.sort(){ a,b -> b?.lastUpdated <=> a?.lastUpdated }}">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can create a method for comparing lastUpdated dates in the domain, and use the .sort() method without passing closure?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest maybe just overriding compareTo method in Comment domain and just use .sort(), but it's optional.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like Kacper suggested, I'll overwrite compareTo and call sort() in the GSP.

<g:link controller="stockMovement" action="comments" params="${[ id: stockMovement?.id ]}">
<warehouse:message code="comments.label" default="Comments"/>
<warehouse:message code="comments.label" default="Comments"/>${fragment}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is ${fragment}?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing how to extract # from the URL, but it didn't. Thanks for noticing that, it needs to be removed.

@@ -12,7 +12,7 @@
</tr>
</thead>
<tbody>
<g:each var="comment" in="${requisition?.comments}">
<g:each var="comment" in="${requisition?.comments?.sort(){ a,b -> b?.lastUpdated <=> a?.lastUpdated }}">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest maybe just overriding compareTo method in Comment domain and just use .sort(), but it's optional.

@@ -28,7 +29,7 @@
<g:if test="${stockMovement?.documents}">
<div class="right">
<div class="button-group">
<g:if test="${stockMovement?.requisition}">
<g:if test="${stockMovement.sourceType == RequisitionSourceType.ELECTRONIC}">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it's only a regression fix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more like an oversight on my part

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use stockMovement.electronicType

Copy link
Collaborator

@awalkowiak awalkowiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside what others noticed, it LGTM (I have a one tiny suggestion in the comments)

@@ -28,7 +29,7 @@
<g:if test="${stockMovement?.documents}">
<div class="right">
<div class="button-group">
<g:if test="${stockMovement?.requisition}">
<g:if test="${stockMovement.sourceType == RequisitionSourceType.ELECTRONIC}">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use stockMovement.electronicType

@@ -555,10 +556,14 @@
<warehouse:message code="documents.label" default="Documents"/>
</g:link>
</li>
<g:if test="${stockMovement?.requisition}">
<li>
<g:if test="${stockMovement.sourceType == RequisitionSourceType.ELECTRONIC}">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@awalkowiak awalkowiak merged commit fb64fef into develop Oct 11, 2023
3 checks passed
@awalkowiak awalkowiak deleted the OBPIH-5825-comments-tab-approvals-comment branch October 11, 2023 08:45
awalkowiak pushed a commit that referenced this pull request Oct 26, 2023
* OBPIH-5825 Add comments count on comments tab
- add comment count badge
- sort comments desc by last updated
- show comments only on requests (check by SourceType == Electronic)

* OBPIH-5825 Overwrite compareTo on Comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants