fix: get_last_email

[uepselon]

Reopens #20692

Description:
The function should return the last email which is part of the timeline according to the creation date of the email. Therefore, it is ordering the communication items in descending order. Since the forEach loop will not break on a return false, it will continue to loop through all items and will return whatever is the last item in the array. I assume this is not as intended. Instead we can use the .every method which breaks on a return false.

Steps to reproduce:

Add multiple email items to a timeline
Call the get_last_email function in form_timeline.js
-> It will return the oldest element of the timeline instead of the newest.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

[uepselon]

Unstale

[sagarvora]

This won't work:

returning nothing (undefined) means that we are returning a falsy value, which will cause the loop to end early:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/every#description

Array.prototype.every is generally used to ensure that every element in your array matches the test you specify.

Just use for..of IMO (you can use the break keyword to end loop):
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

I am converting this to Draft, please fix and mark ready for review.

---

Edit: original author may have expected Array.prototype.forEach to work like jQuery.each which does allow return false:
https://api.jquery.com/jquery.each/

[barredterra]

@sagarvora when testing, it worked just fine. Looks like my test cases were not exhaustive, I see your point. Refactored the function to address your concerns and solve the original bug.

[codecov]

Codecov Report

Merging #20711 (fc520a0) into develop (7042dca) will increase coverage by 0.05%.
The diff coverage is 13.33%.

❗ Current head fc520a0 differs from pull request most recent head c20ac26. Consider uploading reports for the commit c20ac26 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #20711      +/-   ##
===========================================
+ Coverage    63.87%   63.93%   +0.05%     
===========================================
  Files          761      761              
  Lines        68844    68828      -16     
  Branches      6224     6222       -2     
===========================================
+ Hits         43977    44006      +29     
+ Misses       21320    21267      -53     
- Partials      3547     3555       +8     

Flag	Coverage Δ
server-ui	32.16% <22.22%> (-0.06%)	⬇️
ui-tests	51.74% <0.00%> (+0.15%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[phot0n]

What happens in the scenario where there are multiple senders? Does the received communication's "from" gets picked up no matter how many sent communications (from different senders) are in between there?

[barredterra]

What happens in the scenario where there are multiple senders?

I think an email always has exactly one sender.

[phot0n]

I think an email always has exactly one sender.

Ah I'm not talking about multiple senders of an email/communication (they cannot be multiple).

Since communications have the status of sent or received, I'm talking about multiple people sending multiple communications to recipients in the timeline.

[barredterra]

I don't get your question, sorry.

What this function does is to return the most recent email, if any. When the parameter from_recipient is true, it returns the most recent email from the email address contained in the email_field of the current doc, if any.

[sagarvora]

Frappe's style of naming should be preferred. I will fix it.

Suggested change

	const filteredRecords = communications
	const filtered_records = communications