Closed marcaaron closed 2 months ago
I am guessing this is because the file ref needs to be cached outside the component lifecycle or some other strategy to achieve it. Feels like a minor issue that can be easily solved.
No progress yet. Focused on QBO
Gonna take this one External
so we can make some progress.
Triggered auto assignment to @adelekennedy (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Job added to Upwork: https://www.upwork.com/jobs/~014f246ab8eb2607ee
Current assignee @s77rt is eligible for the External assigner, not assigning anyone new.
Group Chat custom avatar not saved when navigating back to invite members page
When creating the group, we use the outdated fileRef.current
which is undefined when coming back.
We should use stashedLocalAvatarImage
which takes the saved picture from the NEW_GROUP_CHAT_DRAFT
Onyx key when creating the groupChat
@neonbhai Thanks for the proposal. Your RCA makes sense but the solution won't work. In onyx we only store the url and we want to pass the whole image.
Still looking for proposals
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
still waiting for proposals - @marcaaron @s77rt is this a more complex issue than originally thought?
@adelekennedy I didn't check in depth but it shouldn't be that hard, no need to adjust the bounty yet
Agree, I think we just haven't found the obvious solution yet - but let's see.
@s77rt @marcaaron @adelekennedy this issue is now 4 weeks old, please consider:
Thanks!
still waiting
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
gonna just keep waiting on this as it's pretty edge-casey. Going to move this to weekly so we don't keep getting dinged
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Still looking for proposals...
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
still pending proposals @neonbhai can you update your proposal with @s77rt feedback?
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Group Chat custom avatar not saved when navigating back to invite members page
The screen remounts every time we navigate, causing the fileRef to be re-initialized upon returning.
We can follow receipt flow to keep the avatar image persist by re-reading stored uri.
In src/pages/NewChatConfirmPage.tsx
change fileRef to state, add avatarFile
as createGroup
dependency and add useEffect
to re-read the stored file when re-mounts.
const [avatarFile, setAvatarFile] = useState<File | CustomRNImageManipulatorResult | undefined>();
..............
useEffect(() => {
if (!stashedLocalAvatarImage) {
return;
}
const onSuccess = (file: File) => {
setAvatarFile(file)
};
const onFailure = () => {
setAvatarFile(undefined);
Report.setGroupDraft({avatarUri: null, avatarFileName: null, avatarFileType: null});
};
FileUtils.readFileAsync(stashedLocalAvatarImage.toString(), newGroupDraft?.avatarFileName ?? '', onSuccess, onFailure, newGroupDraft?.avatarFileType ?? '');
}, []);
We can move fileRef to a context to make it persist while navigate.
Create src/hooks/useFileRef.tsx
Wrap RHP stack navigator with FileRefProvider
<View style={styles.RHPNavigatorContainer(isSmallScreenWidth)}>
<FileRefProvider>
<Stack.Navigator
.....
fileRef
in NewChatConfirmPage
will use useFileRef
function NewChatConfirmPage({newGroupDraft, allPersonalDetails}: NewChatConfirmPageProps) {
const optimisticReportID = useRef<string>(ReportUtils.generateReportID());
const fileRef = useFileRef();
Clean up
const createGroup = useCallback(() => {
if (!newGroupDraft) {
return;
}
const logins: string[] = (newGroupDraft.participants ?? []).map((participant) => participant.login);
Report.navigateToAndOpenReport(logins, true, newGroupDraft.reportName ?? '', newGroupDraft.avatarUri ?? '', fileRef &&fileRef.current, optimisticReportID.current, true);
if (fileRef) {
fileRef.current = undefined;
}
@wildan-m Thanks for the proposal. Your RCA makes sense but the solution is over-engineered. We already have this flow working well with receipts. Let's follow the same approach.
@s77rt receipt has different case, perform back navigation from image preview will force the user to re-upload the scanned receipt, but group chat will expect we re-visit the image preview page without re-uploading the image.
@wildan-m If you start the money request process from the FAB you can go back to the participants page
@s77rt thanks for the hints. I've updated my propsal.
Change log: Move prev solution to alternative, add new main solution that mimic receipt upload behavior.
@wildan-m Thanks for looking into this. The solution looks good to me 👍.
🎀 👀 🎀 C+ reviewed Link to proposal
Triggered auto assignment to @MonilBhavsar, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@MonilBhavsar, do we need to consider anything else before we proceed?
No thanks! looks good to me. Thanks for keeping it consistent with receipt scan flow
📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
📣 @wildan-m 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer 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 📖
@s77rt The PR is ready for review.
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.0-9 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 2024-06-28. :confetti_ball:
For reference, here are some details about the assignees on this issue:
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
Regression Test Proposal
LGTM
Payouts due:
Coming from https://github.com/Expensify/App/pull/39757#issuecomment-2052659550
Expected:
Avatar image should be saved.
Actual
Picture is not taken into consideration.
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @adelekennedy