Closed kbecciv closed 2 years ago
Triggered auto assignment to @roryabraham (Engineering
), see https://stackoverflow.com/c/expensify/questions/4319 for more details.
Triggered auto assignment to @dylanexpensify (External
), see https://stackoverflow.com/c/expensify/questions/8582 for more details.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported
)
π£ @parasharrajat You have been assigned to this job by @melvin-bot[bot]! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
Current assignee @roryabraham is eligible for the Exported assigner, not assigning anyone new.
IOUParticipantsRequest
component calls getNewChatOptions
from OptionsListUtils
, and it calls getOptions
function which sorts contacts by lastVisitedTimestamp
by default.
We should pass sortByLastMessageTimestamp
parameter through these functions:
To do this we should take sortByLastMessageTimestamp
as a parameter and pass it to the getOptions
in the following functions in OptionsListUtils
:
function getSearchOptions(
reports,
personalDetails,
searchValue = '',
betas,
sortByLastMessageTimestamp = false,
) {
return getOptions(reports, personalDetails, 0, {
betas,
searchValue: searchValue.trim(),
includeRecentReports: true,
includeMultipleParticipantReports: true,
maxRecentReportsToShow: 0, // Unlimited
prioritizePinnedReports: false,
prioritizeDefaultRoomsInSearch: false,
sortByReportTypeInSearch: true,
showChatPreviewLine: true,
showReportsWithNoComments: true,
includePersonalDetails: true,
sortByLastMessageTimestamp: !!sortByLastMessageTimestamp,
forcePolicyNamePreview: true,
prioritizeIOUDebts: false,
});
}
// ...
function getNewChatOptions(
reports,
personalDetails,
betas = [],
searchValue = '',
selectedOptions = [],
excludeLogins = [],
sortByLastMessageTimestamp = false,
) {
return getOptions(reports, personalDetails, 0, {
betas,
searchValue: searchValue.trim(),
selectedOptions,
excludeDefaultRooms: true,
includeRecentReports: true,
includePersonalDetails: true,
maxRecentReportsToShow: 5,
excludeLogins,
sortByLastMessageTimestamp,
});
}
We should update OptionsListUtils.getNewChatOptions
function call with the new parameter in the constructor of IOUParticipantsRequest
component:
constructor(props) {
super(props);
this.addSingleParticipant = this.addSingleParticipant.bind(this);
const {
recentReports,
personalDetails,
userToInvite,
} = OptionsListUtils.getNewChatOptions(
props.reports,
props.personalDetails,
props.betas,
'',
[],
CONST.EXPENSIFY_EMAILS,
true, // New parameter
);
this.state = {
recentReports,
personalDetails,
userToInvite,
searchValue: '',
};
}
Before ;
https://user-images.githubusercontent.com/6093207/160303729-c14ee68a-ed8d-4a43-b116-8d35732ae693.mp4
After Fix issue ;
https://user-images.githubusercontent.com/6093207/160303743-3ce8f40c-03f3-4691-944b-0055a13a8bff.mp4
@metehanozyurt's proposal looks good to me.
cc: @roryabraham
Rory, Do we need the same behavior everywhere where we are showing newChat Contacts?
If so, we will not have to add another param to OptionsListUtils.getNewChatOptions
. It will just modify the call to getOptions in the above proposal.
:ribbon: :eyes: :ribbon: C+ reviewed
@metehanozyurt's proposal looks good, but I am unwilling to approve any proposal or PR that touches OptionsListUtils
without also creating unit tests to cover the change.
@metehanozyurt If you agree to also create unit tests, π this message and I will assign you the job.
Oh, yeah. I was thinking the same just forgot to ask here.
Checking in, seems we will have someone for the job @parasharrajat? Just checking in case we need to double the price
No need to double. we are good here.
Hi @parasharrajat and @roryabraham. I add some tests to OptionsListUtilsTest. Hope you like it.
I'm sorry for being late. I wrote before, but I was very nervous because the results I wanted did not come out. Then I noticed that the 'lastMessageTimestamp' values ββwere entered the same value in test file. Then i check the other results too. I wrote test for 'getSearchOptions' and 'getNewChatOptions' functions.
here edited OptionsListUtilsTest file
https://github.com/metehanozyurt/testFile/blob/main/OptionsListUtilsTest.js
I know this issue title about 'Request money' issue but when it will fixed 'Split Money' , 'New Chat' and 'New Group' isues can came up again.
When we change this file (IOUParticipantsRequest), other files will sort differently. IOUParticipantsSplit, NewChatPage,
i can set the parameter if you want.
before ;
https://user-images.githubusercontent.com/6093207/161388237-31a0f0d1-ed5f-46d9-b8e1-797a5fb66c91.mp4
after
https://user-images.githubusercontent.com/6093207/161388479-1bc184a1-fd89-484e-9a0c-02480fe41a04.mp4
@parasharrajat still good with where we are here on price?
@dylanexpensify If we are good to hire the selected proposal then I don't think a price increase is needed.
@metehanozyurt I didn't understand this post https://github.com/Expensify/App/issues/8220#issuecomment-1086657856. Could you please try to rephrase it?
Are you trying to say that, your proposed solution does not have desired results?
No my proposal can fix the problem @parasharrajat. I wanted to say some other pages need to change too for sorting about recent chats. I can do it when i fix the issue if you like no problem for me. I just wanted to avoid possible new issues β€ .
Ok. Thanks for the clarification. Let's wait for Rory's comment. If you need to change other parts of code/pages unrelated to this issue as increased scope, we might issue a bonus for doing that.
@metehanozyurt I think your proposal looks good, and I agree the other pages (IOUParticipantsSplit
, NewChatPage
, etc...) should also sort by timestamp. I did not review the unit tests, but we can always adjust unit tests in the PR. We want them to be thorough but also readable.
I would be happy to issue a $250 bonus for the increased scope, so long as unit tests thoroughly cover all new and old cases.
π£ @metehanozyurt You have been assigned to this job by @roryabraham! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
I am not sure of how the results are sorted under the recent and contacts section on the various pages of the app.
Can someone confirm the sort order of the following pages? cc: @Expensify/engineering
Thank you @parasharrajat if pass true params Search Page
works fine. I'm confused because Workspace Invite Member page
not using resent chats and not using reports too.
I'm working on sorting in alphabetical order solution on Contacts
2 days. I read your message and slack channel π .
So Others seem to agree on the following order.
For pages like search page, workspace invite, request money, and send money Sorted by recently used
@metehanozyurt Let me know if you have any questions. Otherwise, please update the PR to follow the above order.
Thank you so much all your help @parasharrajat π . I have come to the end of the study according to the new updates. I will check the tests and make a new commit .
Thank you.
Hint: You can always create helper methods in OptionListUtils if you want to get options for a different configuration from the rest of the pages.
I just made my new commit @parasharrajat . I will extending test cases. I will shoot videos for all platforms now for all cases. Thank you again π .
I edit test steps and make web video. I will continue but I think some problem on servers I report this situation on slack channel.
any update here @metehanozyurt
I made the PR. I'm waiting for my PR to be reviewed or update requests now. Thank you π
I'm heading on parental leave so reassigning! Thank you to whoever gets assigned this! β€οΈ
Triggered auto assignment to @NicMendonca (External
), see https://stackoverflow.com/c/expensify/questions/8582 for more details.
Congratulations @dylanexpensify β€ . I wish your baby a happy, peaceful and healthy life β¨πππΆ .
PR in review
Issue not reproducible during KI retests. (First week)
Latest PR update here
PR still open
^^ same update
PR was just merged today
Issue not reproducible during KI retests. (Second week)
Going ooo to re-assigning
Triggered auto assignment to @JmillsExpensify (External
), see https://stackoverflow.com/c/expensify/questions/8582 for more details.
deployed to staging, so will be ready for payment soon
Great thanks! I'll handle the payment once we clear the regression period.
This issue might be affected by the late production deploy. PR still isn't showing we're in the regression period.
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.1.79-17 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2022-07-15. :confetti_ball:
Issue not reproducible during KI retests. (Third week)
Almost through the regression period following the production deploy last week.
Looks like we're just awaiting payment here
Issue not reproducible during KI retests. (4th week)
@roryabraham yes indeed. I just took a look at the Upwork, and we actually still need to hire contributors. I'm going to jump in and send offers now, such that when I'm up tomorrow contracts will be accepted and I can issue payments.
If you havenβt already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Pre-Condition: Have a couple/several chats already open with other accounts
Expected Result:
The user expects to see five most recent chats appear under the
Recents
header (Excluding group chats and concierge chat)Actual Result:
The user sees five chats in this section, however they are not the five most recent chats. There is one chat that should not be placed within this list
Workaround:
Unknown
Platform:
Where is this issue occurring?
Version Number: 1.1.44.1
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester: applausetester+ebezerra@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/93399543/158920581-9767c787-aed1-4e4e-89ff-00372749e044.mp4
https://user-images.githubusercontent.com/93399543/158920663-5375bb46-2fba-4e73-8bdd-3ee0d82f319e.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause
Slack conversation:
View all open jobs on GitHub