fix: update CSV upload options for Members#65398
Conversation
|
@sobitneupane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
I can't record in native because of the app crash, will add later. And I'll fix the typescript check soon, it's related to translation |
|
@nkdengineer @sobitneupane is this ready to review now? Can we prioritize this one as it's for a larger customer. |
|
Sure @danielrvidal @nkdengineer Could you please fix typescript checks? |
|
@nkdengineer Could you please add a CSV test file as well? |
|
@nkdengineer We would only like to add the columns to the existing “Import Spreadsheet” feature on the Members page.
|
|
thanks for your review @sobitneupane, i'll give an update soon |
@sobitneupane I fixed |
|
@sobitneupane Except for |
|
@nkdengineer You probably missed this
|
|
@sobitneupane here you go |
|
@nkdengineer After importing the spreadsheet, we should display the approvers in the workflow. In this comment, I only meant that we don’t need to add an "Import approval workflow" button on the Workflow page. However, I do believe we want to show the imported members in the workflow section.
@danielrvidal Could you please confirm this? |
|
Correct, if the approval flows are updated from columns being uploard in the members page, they would need to be accounted for in the workflows page. |
|
Yes, for sure. |
|
@sobitneupane @nkdengineer let me know if there are any questions before getting started on this one. |
|
@nkdengineer Could you please update the PR to address this comment? |
|
@sobitneupane i'm checking now |
|
@sobitneupane it seems that for users in the |
|
@rlinoz cc'ing you in here for eyes to help. I haven't bee subscribed to this PR to date!
Yeah, this is just how the spreadsheet for import works and has done since the dawn of time. Every user being added to the workspace has to be in the |
|
@tgolen Updated all your suggestions. |
|
Aw got it, I'll wait for that. My bad I thought it was out. |
tgolen
left a comment
There was a problem hiding this comment.
Thanks! Just a couple small things
| // eslint-disable-next-line react-compiler/react-compiler | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
Co-authored-by: Tim Golen <tgolen@gmail.com>
|
@nkdengineer can you please merge main again so perf tests pass? |
|
Done. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/rlinoz in version: 9.1.98-0 🚀
|
|
Hello, I tested this PR and have some doubts. Because I think it can be exploited if the Advance Approval can be set via uploading the CSV file via the Import Member without upgrading the Plan. video.PR65398.mp4Thanks |
Could you please provide the answer @nkdengineer ? |
Nice catch, they shouldn't be allowed to configure advanced approvals on the 2025-08-26_11-02-04.mp4 |
|
@trjExpensify In the collect workspace, after we upload a CSV with |
|
Ah, so you're saying:
Is that, right? If so, then in NewDot I would expect to:
CC: @JmillsExpensify |
Yes. |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.1.98-12 🚀
|
|
I feel like what @trjExpensify is expecting would work well. I think that will make this functional for most cases and if we end up wanting to add some polish where if the user adds advanced approvals while on Collect, we can have better messaging. But I think I'd probably add that later and only if we hear confusion. I feel like that will be more of an edge case than something a lot of people are hitting. If you are managing your approvals with CSVs , you likely hit the Control upgrade far before that. |
| const employee = policy?.employeeList?.[curr.email]; | ||
| if (employee) { | ||
| if (curr.role !== employee.role) { | ||
| if (curr.role !== employee.role || curr.submitsTo !== employee.submitsTo || curr.forwardsTo !== employee.forwardsTo) { |
| </PressableWithoutFeedback> | ||
| </FixedFooter> | ||
| <ConfirmModal | ||
| isVisible={spreadsheet?.shouldFinalModalBeOpened} |
There was a problem hiding this comment.
This second modal caused duplicate modal to appear, which caused #74907
|
This case was not handled in the PR. |

Explanation of Change
Fixed Issues
$ #65056
PROPOSAL: #65056 (comment)
Tests
email, submitTo, approveTo, rolecolumns (all members have the role column)email, submitTo, approveTo, rolecolumns (at least one member doesn't have the role column)Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2025-07-31.at.15.01.03.mov
Screen.Recording.2025-07-31.at.14.43.00.mov
Android: mWeb Chrome
android-mweb.mov
Screen.Recording.2025-07-31.at.14.53.00.mov
iOS: Native
Screen.Recording.2025-07-31.at.15.00.18.mov
Screen.Recording.2025-07-31.at.14.51.56.mov
iOS: mWeb Safari
ios-mweb.mov
Screen.Recording.2025-07-31.at.14.53.40.mov
MacOS: Chrome / Safari
web-resize.mp4
Screen.Recording.2025-07-31.at.14.40.14.mov
MacOS: Desktop
desktop.mov
Screen.Recording.2025-07-31.at.14.56.03.mov