Migrate Files table to data grid #1438

chiaberry · 2024-10-01T22:08:27Z

Associated issues

cityofaustin/atd-data-tech#18176
and a bonus cityofaustin/atd-data-tech#17597

I copied the patterns laid out in the Funding Table refactor, using DataGrid instead of Material Table, memoizing the columns, implementing the row mode model that data grid needs etc.

Files table had some data validation, the DataGrid column definition has a property preProcessEditCellProps that checks the input and will sets the error parameter update The file_url field also depends on the file_key field, I added a function to validate against the different variations in data.
the Files table also had some helperText in the text input fields
I added the Delete Confirmation Modal to the Files Table delete action
I wrote about this in a comment but I ran into the nuances between the valueGetter vs valueFormatter functions, the valueGetter function changes the value, it will then be used the sort comparator. valueFormatter does not change the value, but formats it for rendering and the sort comparator uses the original value. Our file types table has values 1, 2, 3, 4, that correspond to ["", "Funding", "Plans", "Estimates", "Other"].

Testing

URL to test:
There arent too many projects with files in staging, but here is one:
https://deploy-preview-1438--atd-moped-main.netlify.app/moped/projects/1695?tab=files

or if you run locally with prod data, this is a good project with files
https://localhost:3000/moped/projects/437?tab=files

Steps to test:

Test creating a file (use a url or fake file name to avoid issues with the file upload process in staging/local)
Test editing a record. You can do this by clicking the pencil, or double clicking in an editable field (name, file, type, description)
Cancel your edits. Or save your edits!!
Try deleting a record. A confirmation modal pops up.

Test sorting on file type. it sorts ascending, descending and then unsorts.

Do you like the widths of the columns as they are defined now? You can resize now, but wondering if anything should be defined differently.

Ship list

Code reviewed
Product manager approved
Product manager checked DB view dependencies
~~- [ ] Product manager added to QA test script if applicable~~

johnclary

Thank you for knocking out another Material Table instance. I'm running into some bugginess that I'm sure will be a quick fix 🙏 🙌

johnclary · 2024-10-04T15:44:35Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

@@ -56,33 +51,225 @@ const useStyles = makeStyles((theme) => ({
  },
  downloadLink: {
    cursor: "pointer",
+    overflow: "hidden",
+    display: "block",
+    textOverflow: "ellipsis",


i'm not sure if this is working quite right—here's a long file link in project 437

ah because this isnt a link, this is just text. If it's truncated or overflow hidden, then users wouldn't be able to read the entire path

I'm seeing a different issue when I entered a really long fake G drive location.

Here is an example of the behavior that I see in staging with the same length network location - https://moped.austinmobility.io/moped/projects/1695?tab=files.

Edit: I also realized that it is the codeStyle styles below that handle non-links and needs to be updated. I requested a change there instead of here.

johnclary · 2024-10-04T15:51:00Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+        preProcessEditCellProps: (params) => {
+          const hasError = params.props.value.trim().length < 1;
+          return { ...params.props, error: hasError };
+        },


i'm not sure if this is source of the bug, but i'm hitting this error when i edit any column 💥

it is the source, thanks for catching it -- the weird thing is that I see the bug locally, but I am not seeing it on the netlify deploy link, which is confusing the heck out of me

Part of the cause for this error message was my own lazy code, but the other part is that in cases where someone uploaded a file, the file_url was always going to be null, which was always returning an error and preventing the record from being updated.

johnclary · 2024-10-04T15:52:56Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+        width: 150,
+        valueGetter: (value) => fileTypes[value],
+        renderEditCell: (props) => <ProjectFilesTypeSelect {...props} />,
+      },


This input is a little squished (like half the width of the column) in edit mode. Maybe there are some tweaks needed in the Select component?

I am also getting the dreaded autocomplete warning when i switch into edit mode

I broke this when I added the valueGetter to make sorting work ☹️ . I am going to see how to fix this but also maintain ability to sort by the words and not the numbers

I broke the select when I used valueGetter, valueFormatter changes the value only for rendering and doesn't affect the underlying value. which means you still cant sort. valueGetter affects the underlying value, which means we can sort alphabetically vs 1,2,3,4 but it was then passing the string to the select component instead of the file_type number.

So now its valueFormatter to render the file_type strings, and passing those strings into the data grid sort comparator, while keeping the original value so the select works.

thanks for fixing and explaining the difference 🙏 I tested the select and the sorting, and I see no warning and the sort is working. 🚀

ditto—great to have these details ironed out for the next MaterialTable that gets replaced ✅

johnclary · 2024-10-04T15:58:44Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+    handleCancelClick,
+    handleEditClick,
+    handleDeleteOpen,
+  ]);


love the memoized columns—looks great!!

mddilley

Yay - this is looking great! I tested locally, and I was able to create linked files and edit from within the table rows. I'm requesting one change based on what I was seeing with long network locations for files that are links.

mddilley · 2024-10-07T21:50:30Z

moped-editor/src/queries/project.js

@@ -682,7 +682,7 @@ export const PROJECT_FILE_ATTACHMENTS_UPDATE = gql`
    $fileId: Int!
    $fileName: String!
    $fileType: Int!
-    $fileDescription: String!


🐛 squashed! 🙏

mddilley · 2024-10-07T21:51:33Z

moped-editor/src/views/projects/projectView/DataGridTextField.js

nice to see this component growing up to handle validation! 🌱

mddilley · 2024-10-07T21:57:50Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

@@ -56,33 +51,225 @@ const useStyles = makeStyles((theme) => ({
  },
  downloadLink: {
    cursor: "pointer",
+    overflow: "hidden",
+    display: "block",
+    textOverflow: "ellipsis",


I'm seeing a different issue when I entered a really long fake G drive location.

Here is an example of the behavior that I see in staging with the same length network location - https://moped.austinmobility.io/moped/projects/1695?tab=files.

Edit: I also realized that it is the codeStyle styles below that handle non-links and needs to be updated. I requested a change there instead of here.

moped-editor/src/views/projects/projectView/ProjectFiles.js

mddilley · 2024-10-07T22:47:52Z

moped-editor/src/views/projects/projectView/ProjectFiles.js

+        width: 150,
+        valueGetter: (value) => fileTypes[value],
+        renderEditCell: (props) => <ProjectFilesTypeSelect {...props} />,
+      },


thanks for fixing and explaining the difference 🙏 I tested the select and the sorting, and I see no warning and the sort is working. 🚀

moped-editor/src/views/projects/projectView/ProjectFiles.js

johnclary

Nice—it's working great! Two minor things that are not blocking my approval:

When I edit a row, sometimes the sort order changes after I save. That was tripping me up a bit because i thought my edits were not getting saved correctly. I'm not sure if this is a bug in prod, so I'll let y'all decide on it 👍

Also, I noticed the activity logs for my edits were missing a field or two—possibly audit fields? Anyway, it's probably not a new bug.

🚢

mddilley

Looks amazing! Thanks for updating the sort and handling the network locations wrapping too. So excited to have another table converted to DataGrid, and there are nice improvements in here too! It is going to make the next one even more straightforward. 🙌 🚢

mddilley · 2024-10-08T21:46:27Z

fyi - i created this spin off issue to capture another small bug that is in production too. sorting the "File" column doesn't work quite as expected on uploaded files since they all have a common S3 path. Plenty of improvements in this PR so let's save for another day! Thank you again @chiaberry! 🎉

tillyw

tested locally and looks and functions great! thank you for upgrading this table with DataGrid and adding the increased functionality, validation and modal! being able to inline edit is so nice ✨ long file paths and urls render as expected:

a couple minor things we could maybe address in the future:

is it possible to disable editing on the 'file' field when it is an actual file (as opposed to a url or file path) since you can't actually edit that field in that case?
i know we've come across this before, but when i double click on the 'file' field when it is a url it tries to open the url rather than allowing me to edit the field

as far as default column widths, i think we could shrink the 'file size' field by at least half and give that extra space to the 'file' field, since file size will probably only ever be around 5-6 characters

mateoclarke

👍

chiaberry · 2024-10-10T13:45:52Z

@tillyw thank you for pointing out the file size column width could be smaller!
and thanks everyone for the reviews

chiaberry added 9 commits September 30, 2024 13:06

the data is rendering

9b5dfb3

add the toolbar and more of the edits

8eec112

linting

3b32597

delete a row

33891f7

able to edit a row

5795234

validate input in data grid

4031490

remove older props

4c4effe

ability to edit the type select in data grid

31a99ba

looks like we dont need the field selector

d27ceb9

chiaberry added the WIP Work in progress label Oct 1, 2024

chiaberry added 5 commits October 2, 2024 14:41

remove the unneeded text field

06ab1bc

fix external link rendering

ed304bd

add delete confirmation modal to files table

924295e

tabbing seems to work without the handler

5831ccb

update comment

bf5c6b8

chiaberry removed the WIP Work in progress label Oct 3, 2024

chiaberry requested review from mddilley, frankhereford, mateoclarke, johnclary, tillyw and roseeichelmann October 3, 2024 19:04

updating the value getter so the sort workds

65f27a2

johnclary requested changes Oct 4, 2024

View reviewed changes

chiaberry added 2 commits October 4, 2024 12:31

Merge branch 'main' into 18176-data-grid-files

610c3c4

check if value exists, dont try to trim null

44b9852

chiaberry added the WIP Work in progress label Oct 4, 2024

chiaberry added 3 commits October 7, 2024 11:19

description is not required

2c9623c

fill width more or less of parent component

b2f79e3

clean up payload

19fdb86

chiaberry added 2 commits October 7, 2024 11:59

use data grid sort comparator

82c3335

write a handler to validate file input

6348377

chiaberry removed the WIP Work in progress label Oct 7, 2024

chiaberry requested a review from johnclary October 7, 2024 19:00

mddilley requested changes Oct 7, 2024

View reviewed changes

mddilley mentioned this pull request Oct 7, 2024

Project files activity log is not rendering correctly and showing an extra comma cityofaustin/atd-data-tech#17597

Closed

break the words

1dc2d3b

johnclary approved these changes Oct 8, 2024

View reviewed changes

chiaberry added 2 commits October 8, 2024 08:52

ignore the audit fields in in the files activity log

c0fbe1e

order files by created at desc

d42a31a

mddilley approved these changes Oct 8, 2024

View reviewed changes

tillyw approved these changes Oct 9, 2024

View reviewed changes

mateoclarke mentioned this pull request Oct 9, 2024

Moar <FormattedDateString> #1450

Merged

4 tasks

mateoclarke approved these changes Oct 10, 2024

View reviewed changes

give the file size column a size

fd1ce13

chiaberry merged commit a50f7da into main Oct 10, 2024
4 checks passed

mddilley mentioned this pull request Oct 16, 2024

Migrate project team table to DataGridPro cityofaustin/atd-data-tech#18178

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Files table to data grid #1438

Migrate Files table to data grid #1438

chiaberry commented Oct 1, 2024 •

edited by mddilley

Loading

johnclary left a comment

johnclary Oct 4, 2024

chiaberry Oct 4, 2024

mddilley Oct 7, 2024

johnclary Oct 4, 2024

chiaberry Oct 4, 2024

chiaberry Oct 7, 2024

johnclary Oct 4, 2024

chiaberry Oct 4, 2024

chiaberry Oct 7, 2024

mddilley Oct 7, 2024

johnclary Oct 8, 2024

johnclary Oct 4, 2024

mddilley left a comment

mddilley Oct 7, 2024

mddilley Oct 7, 2024

mddilley Oct 7, 2024

mddilley Oct 7, 2024

johnclary left a comment

mddilley left a comment

mddilley commented Oct 8, 2024

tillyw left a comment •

edited

Loading

mateoclarke left a comment

chiaberry commented Oct 10, 2024

Migrate Files table to data grid #1438

Migrate Files table to data grid #1438

Conversation

chiaberry commented Oct 1, 2024 • edited by mddilley Loading

Associated issues

Testing

Ship list

johnclary left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mddilley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnclary left a comment

Choose a reason for hiding this comment

mddilley left a comment

Choose a reason for hiding this comment

mddilley commented Oct 8, 2024

tillyw left a comment • edited Loading

Choose a reason for hiding this comment

mateoclarke left a comment

Choose a reason for hiding this comment

chiaberry commented Oct 10, 2024

chiaberry commented Oct 1, 2024 •

edited by mddilley

Loading

tillyw left a comment •

edited

Loading