Page MenuHomePhabricator

[lib] refactor handling new inconsistencies in report store reducer for using operations
ClosedPublic

Authored by kamil on May 23 2023, 1:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 8, 4:52 AM
Unknown Object (File)
Wed, Mar 6, 9:51 AM
Unknown Object (File)
Feb 20 2024, 12:34 AM
Unknown Object (File)
Feb 20 2024, 12:24 AM
Unknown Object (File)
Feb 19 2024, 7:17 PM
Unknown Object (File)
Feb 19 2024, 2:03 PM
Unknown Object (File)
Feb 19 2024, 11:54 AM
Unknown Object (File)
Feb 5 2024, 2:42 AM
Subscribers

Details

Summary

Convert logic for ops approach.

Depends on D7932

Test Plan

Tests

Diff Detail

Repository
rCOMM Comm
Branch
land-rs
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.May 23 2023, 1:58 AM
tomek added inline comments.
lib/reducers/report-store-reducer.js
148–164 ↗(On Diff #26844)

It's quite strange that this happens outside the actions, but it follows the convention from the original code.

This revision is now accepted and ready to land.May 24 2023, 3:08 AM

rebase

lib/reducers/report-store-reducer.js
148–164 ↗(On Diff #26844)

Yeah, but it'll be hard to fix. That's because some inconsistencies are not added to store by dispatching action but directly from other reducers as reduceReportStore argument

lib/reducers/report-store-reducer.js
148 ↗(On Diff #27145)

I think this check should've been newReports.length > 0. As a result, we end up changing reportStore on every since action. Looking at the code, this shouldn't have a very negative effect (I don't think it causes any components to rerender, since processReportStoreOperations keeps state.queuedReports if reportStoreOperations is empty). However, it is causing ENG-5694