Open IuliiaHerets opened 2 weeks ago
Triggered auto assignment to @alexpensify (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.
mWeb - Category - Categories imported are disabled
It's because after we upload the spreadsheet, the enabled value column return value boolean instead of string
So since we check if the enabled column is equal to true
it will return false
https://github.com/Expensify/App/blob/4d9be4eb2537b49fa6cf35135770cf3c6d62a1cd/src/pages/workspace/categories/ImportedCategoriesPage.tsx#L100
We should convert the enabled column value to string
return {
name,
// enabled: categoriesEnabledColumn !== -1 ? categoriesEnabled?.[containsHeader ? index + 1 : index] === 'true' : true,
enabled: categoriesEnabledColumn !== -1 ? categoriesEnabled?.[containsHeader ? index + 1 : index]?.toString() === 'true' : true,
https://github.com/user-attachments/assets/bd361d44-aab6-4ce9-af2d-5ec9fe57f46c
Edited by proposal-police: This proposal was edited at 2024-11-13 14:32:39 UTC.
Categories imported are disabled
After we read the CSV spreadsheet into JSON, the type of the 'enable' field is returned as boolean data https://github.com/Expensify/App/blob/8d3f1db0ef1b28c5ef9c03f251d9b9dc2adaed4e/src/components/ImportSpreadsheet.tsx#L94-L97
The condition check for enable will be false because we are checking for the string 'true', which is a boolean as a string https://github.com/Expensify/App/blob/1dd6f993f1e479d4eac6dd3de355510ed8257093/src/pages/workspace/categories/ImportedCategoriesPage.tsx#L100
To fix this issue, we should convert the data in the cell to a string type to match the expected type and the current logic check. We should use this approach to fix any mismatches, and for the rest of the fields, we should force the return to be of type string[][].
// src/components/ImportSpreadsheet.tsx#L96
.then((arrayBuffer) => {
const workbook = XLSX.read(new Uint8Array(arrayBuffer), {type: 'buffer'});
const worksheet = workbook.Sheets[workbook.SheetNames[0]];
const data = XLSX.utils.sheet_to_json(worksheet, {header: 1, blankrows: false});
+ const convertedData = data.map((row) => row.map((cell) => (typeof cell === 'string' ? cell : String(cell))));
- setSpreadsheetData(data as string[][])
+ setSpreadsheetData(convertedData as string[][])
.then(() => {
Navigation.navigate(goTo);
})
.catch(() => {
setUploadFileError(true, 'spreadsheet.importFailedTitle', 'spreadsheet.invalidFileMessage');
});
})
The main solution fixes the issue where the fields return the wrong data type for all entries. If we want to focus only on enable, it can be changed like this:
// src/pages/workspace/categories/ImportedCategoriesPage.tsx#L93
- const categoriesEnabled = categoriesEnabledColumn !== -1 ? spreadsheet?.data[categoriesEnabledColumn].map((enabled) => enabled) : [];
+ const categoriesEnabled =
+ categoriesEnabledColumn !== -1 ? spreadsheet?.data[categoriesEnabledColumn].map((enabled) => (typeof enabled === 'string' ? enabled : String(enabled))) : [];
Job added to Upwork: https://www.upwork.com/jobs/~021858671038820315324
Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External
)
@getusha, when you get a chance, please review these proposals and identify whether one will fix the issue. Thank you!
Category - Categories imported are disabled
It appears that the category file associated with this issue was exported from another source, resulting in the enable field being saved as a boolean type. Typically, when exporting categories on Expensify, all data in the file should be stored as string type.
This discrepancy causes the above condition to not match and the enable field is stored as false.
When importing a spreadsheet, we should ensure that all data is converted to string type exclude null or undefined values
const stringData = data.map(row => row.map(cell => (cell !== null && cell !== undefined ? String(cell) : cell)));
Please note: If we donโt exclude null and undefined values, they will be saved as strings, such as 'null' and 'undefined'. This could potentially break the UI and result in unexpected behavior. My approach is largely similar to the proposal above; however, I believe it offers significant improvements and mitigates many risks, particularly because ImportSpreadsheet is utilized in several other places.
Why convert it to string if it's always a boolean, can't we just use that?
Typically, when exporting categories on Expensify, all data in the file should be stored as string type.
Therefore, when importing the category file, we handle all data to string type
cc @getusha
Yes we can also check if the value is equal to boolean as well i.e ... === true
but as you can see the type of spreadsheet data is number, string
@getusha We can handle boolean, number, or other types. However, with the current logic, we are handling string[][]. To align the data with the expected type, we should convert it accordingly to minimize changes and updates in the component logic. Therefore, I believe converting the data is a better approach for this ticket.
@alexpensify, @getusha Eep! 4 days overdue now. Issues have feelings too...
๐ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐ธ
@alexpensify @getusha this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@getusha when you get a chance, can you please review the recent feedback? Thanks!
@alexpensify, @getusha Still overdue 6 days?! Let's take care of this!
Reviewing
It makes sense to fix it in ImportSpreadsheet
since there are other pages like ImportedTagsPage
where the issue could possibly occur. @huult's proposal looks good to me
๐ ๐ ๐ C+ Reviewed.
Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
๐ฃ @huult ๐ 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 ๐
If you havenโt already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.61-0 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
Categories imported must be shown as enabled.
Actual Result:
Categories imported are shown as disabled.
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6663366_1731473053062!Applausecard_s_Workspace_categories.csv
https://github.com/user-attachments/assets/cd7f6572-3e6e-46f9-b9c1-50cb5a47f6c5
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @getusha