Skip to content

Conversation

@giovannidavi
Copy link

@giovannidavi giovannidavi commented Feb 19, 2021

Description

Check List

If not relevant to pull request, check off as complete

  • All tests passing
  • Docs updated with any changes or examples if applicable
  • Added tests to ensure new feature(s) work properly

Relevant Issues

#294

@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #324 (5c8408f) into master (9622158) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #324      +/-   ##
==========================================
+ Coverage   89.98%   90.00%   +0.01%     
==========================================
  Files          19       19              
  Lines         629      630       +1     
==========================================
+ Hits          566      567       +1     
  Misses         63       63              
Impacted Files Coverage Δ
src/reducers/dataReducer.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9622158...5c8408f. Read the comment docs.

`${collection}.${doc}.newData.field`,
data[doc].newData.field,
);
expect(result).to.not.have.nested.property(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was in place for a reason - not removing a value which is removed from the object will cause stale data in certain situations

);
}
// Otherwise merge with existing data
const mergedData = assign(previousData, data);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will prevent removing a param from an object that is removed elsewhere - what is the goal here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is meant to revert back an existing functionality that has been later removed (while kept in rrf - firebase), data merging.
When for a given collection you change the query to fetch new data (or next page for that matters) new data should be merged with existing data, at the moment is getting replaced.
What the PR is doing is just reverting back the files to their previous status (as mentioned in the issue open #294)

@prescottprue prescottprue changed the title Fix/revert merged data removal feat(dataReducer): revert merged data removal Jan 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants