Merge continue

[imnasnainaec]

Resolves #765

---

This change is 

[codecov-io]

Codecov Report

Merging #766 into master will decrease coverage by 0.00%.
The diff coverage is 20.00%.

@@            Coverage Diff             @@
##           master     #766      +/-   ##
==========================================
- Coverage   51.17%   51.16%   -0.01%     
==========================================
  Files         235      235              
  Lines        6238     6240       +2     
  Branches      405      404       -1     
==========================================
+ Hits         3192     3193       +1     
- Misses       2748     2749       +1     
  Partials      298      298              

Flag	Coverage Δ
#backend	55.30% <ø> (ø)
#frontend	47.32% <20.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ergeDupGoal/MergeDupStep/MergeDupStepComponent.tsx	13.20% <ø> (ø)
.../MergeDupGoal/MergeDupStep/MergeDupStepReducer.tsx	51.94% <0.00%> (-2.11%)	⬇️
.../MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx	79.66% <75.00%> (+1.50%)	⬆️

---

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 bdb341f...6e92073. Read the comment docs.

[imnasnainaec]

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @imnasnainaec)

---

src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx, line 345 at r2 (raw file):

Quoted 8 lines of code…

    // a merge is an identity if all of its senses come from parent
    // and it has the same number of senses as parent
    if (!children.find((val) => val.wordID !== wordID)) {
      if (children.length === data.words[wordID].senses.length) {
        // if the merge is an identity don't bother sending a merge
        return mapping;
      }
    }

Quoted 10 lines of code…

    // a merge is an identity if all of its senses come from parent
    // and it has the same number of senses as parent
    if (
      children.length === 1 &&
      children[0].wordID === wordID &&
      children[0].senses.length === data.words[wordID].senses.length
    ) {
      // if the merge is an identity don't bother sending a merge
      return mapping;
    }

@jasonleenaylor This is the logic change of the PR.

[jasonleenaylor]

Reviewed 5 of 5 files at r1.
Reviewable status: 2 of 5 files reviewed, 2 unresolved discussions (waiting on @imnasnainaec and @jasonleenaylor)

---

src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx, line 340 at r2 (raw file):

      children.length === 1 &&
      children[0].wordID === wordID &&
      children[0].senses.length === data.words[wordID].senses.length

So it doesn't look like the comment and the code agree. (not that it did before)

[jasonleenaylor]

Reviewed 3 of 3 files at r2, 2 of 2 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @imnasnainaec)

---

src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx, line 340 at r2 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

      children.length === 1 &&
      children[0].wordID === wordID &&
      children[0].senses.length === data.words[wordID].senses.length

So it doesn't look like the comment and the code agree. (not that it did before)

It was looking better, until you added the new condition. 😄

[imnasnainaec]

Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

---

src/goals/MergeDupGoal/MergeDupStep/MergeDupStepActions.tsx, line 340 at r2 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

It was looking better, until you added the new condition. 😄

Yeah... the new condition isn't as pretty, but is necessary for such cases as a merging of senses within a single word.

[johnthagen]

Reviewed 2 of 2 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved