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)
Sun, Oct 27, 8:45 PM
Unknown Object (File)
Fri, Oct 4, 1:00 PM
Unknown Object (File)
Sep 29 2024, 1:18 AM
Unknown Object (File)
Sep 29 2024, 1:18 AM
Unknown Object (File)
Sep 29 2024, 1:17 AM
Unknown Object (File)
Sep 29 2024, 1:17 AM
Unknown Object (File)
Sep 29 2024, 1:10 AM
Unknown Object (File)
Sep 15 2024, 2:19 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