mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.55k stars 1.33k forks source link

[data grid] `processRowUpdate` should give access to the `id` of the row when `getRowId()` is used #13381

Closed TeoAvignon closed 1 month ago

TeoAvignon commented 5 months ago

Related page

https://mui.com/x/react-data-grid/editing/#the-processrowupdate-callback

Kind of issue

Missing information

Issue description

Currently processRowUpdate exposes newRow and oldRow but not the internalId of the row used by DataGrid. When getRowId() is not used, it is easy to get the id with newRow.id. However, when it is used, it can be harder to retrieve the internal ID used.

For example, in your Bulk editing example, you use the newRow.id to get the id, but the example would not work if using getRowId(). image

https://mui.com/x/react-data-grid/editing/#the-processrowupdate-callback

Context

Acces easily to internal rowId in processRowUpdate. I think this can also concerns other callbacks exposed by the component. The user can still uses getRowId(), but i do not think it is the proper way to do it

Search keywords: processRowUpdate

michelengelen commented 5 months ago

Hey @TeoAvignon ... thanks for raising this. It is indeed a bit misleading. The example would still work, since the id property on the row model will still be there. Even with using the getRowId prop it would still work. Only when you do not have a unique identifier on your row model and use getRowId you could still adjust this a bit:

const processRowUpdate = React.useCallback<NonNullable<DataGridProps['processRowUpdate']>>(
  (newRow, oldRow) => {
-   const rowId = newRow.id;
+   const rowId = apiRef.current.getRowId(newRow);

    unsavedChangesRef.current.unsavedRows[rowId] = newRow;
    if (!unsavedChangesRef.current.rowsBeforeChange[rowId]) {
      unsavedChangesRef.current.rowsBeforeChange[rowId] = oldRow;
    }
    setHasUnsavedRows(true);
    return newRow;
  },
  [apiRef],
);

Is there a specific use case you have an issue with regarding this? We might be able to assist you in fixing it.

github-actions[bot] commented 5 months ago

The issue has been inactive for 7 days and has been automatically closed.

TeoAvignon commented 5 months ago

Thanks for your quick reply!

In V5 the apiRef does not have getRowId(). I am indeed working with getRowId(). The ticket was about raising this in case you also want to include the internal id ! :)

michelengelen commented 5 months ago

well, we could consider this for v7, but v5 is not in active support anymore. And with getRowId we do already have a workaround.

WDYT @mui/xgrid ?

cherniavskii commented 5 months ago

Passing rowId to processRowUpdate makes sense to me, let's add it 👍🏻

github-actions[bot] commented 1 month ago

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

[!NOTE] @TeoAvignon How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.