<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/contributor-success/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Work on this issue](https://contributors.gitlab.com/manage-issue?action=work&projectId=278964&issueIid=205190) - [Close this issue](https://contributors.gitlab.com/manage-issue?action=close&projectId=278964&issueIid=205190) </details> <!--IssueSummary end--> The following discussion from !23274 should be addressed: - [ ] @pslaughter started a [discussion](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23274#note_286550251): > **suggestion (non-blocking):** This business logic is a little confusing... It seems like under certain conditions, the `diffFiles` will never be updated, but it just so happens that another mutation `SET_DIFF_DATA_BATCH` (which is **very** similar to this one) is actually handling the work. > > We could probably benefit from having a `SET_DIFF_META_DATA` which updates everything but the `diffFiles`. We could then call this new mutation in `fetchDiffFilesMeta` and remove this conditional along with deduping `SET_DIFF_DATA_BATCH` and `SET_DIFF_DATA`. > > <details> > <summary>Untested patch</summary> > > ```patch > diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js > index 31708cc4aa7..5964e634b73 100644 > --- a/app/assets/javascripts/diffs/store/actions.js > +++ b/app/assets/javascripts/diffs/store/actions.js > @@ -84,7 +84,7 @@ export const fetchDiffFiles = ({ state, commit }) => { > commit(types.SET_LOADING, false); > > commit(types.SET_MERGE_REQUEST_DIFFS, res.data.merge_request_diffs || []); > - commit(types.SET_DIFF_DATA, res.data); > + commit(types.SET_DIFF_DATA_BATCH, res.data); > > worker.postMessage(state.diffFiles); > > @@ -163,7 +163,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { > delete strippedData.diff_files; > commit(types.SET_LOADING, false); > commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []); > - commit(types.SET_DIFF_DATA, strippedData); > + commit(types.SET_DIFF_META_DATA, strippedData); > > worker.postMessage(prepareDiffData({ diff: data, priorFiles: state.diffFiles })); > > diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js > index 2097c8d3655..46d3dd32ef3 100644 > --- a/app/assets/javascripts/diffs/store/mutation_types.js > +++ b/app/assets/javascripts/diffs/store/mutation_types.js > @@ -2,7 +2,7 @@ export const SET_BASE_CONFIG = 'SET_BASE_CONFIG'; > export const SET_LOADING = 'SET_LOADING'; > export const SET_BATCH_LOADING = 'SET_BATCH_LOADING'; > export const SET_RETRIEVING_BATCHES = 'SET_RETRIEVING_BATCHES'; > -export const SET_DIFF_DATA = 'SET_DIFF_DATA'; > +export const SET_DIFF_META_DATA = 'SET_DIFF_META_DATA'; > export const SET_DIFF_DATA_BATCH = 'SET_DIFF_DATA_BATCH'; > export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE'; > export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS'; > diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js > index 5a9642cca74..fb1fe96abb6 100644 > --- a/app/assets/javascripts/diffs/store/mutations.js > +++ b/app/assets/javascripts/diffs/store/mutations.js > @@ -44,19 +44,9 @@ export default { > Object.assign(state, { retrievingBatches }); > }, > > - [types.SET_DIFF_DATA](state, data) { > - let files = state.diffFiles; > - > - if ( > - !(gon?.features?.diffsBatchLoad && window.location.search.indexOf('diff_id') === -1) && > - data.diff_files > - ) { > - files = prepareDiffData({ diff: data, priorFiles: files }); > - } > - > + [types.SET_DIFF_META_DATA](state, { diff_files, ...data }) { > Object.assign(state, { > ...convertObjectPropsToCamelCase(data), > - diffFiles: files, > }); > }, > > > ``` > > </details> > > I know this isn't related to this MR. Just leaving some thoughts... :smile: > > Can we create a follow up issue for this?