Protect words/senses imported with data The Combine doesn't handle

[imnasnainaec]

Resolves #1761

---

This change is 

[codecov-commenter]

Codecov Report

Patch coverage: 63.63% and project coverage change: +0.15 🎉

Comparison is base (45e95ee) 49.65% compared to head (d6f3982) 49.81%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2019      +/-   ##
==========================================
+ Coverage   49.65%   49.81%   +0.15%     
==========================================
  Files         293      294       +1     
  Lines        9151     9184      +33     
  Branches      663      666       +3     
==========================================
+ Hits         4544     4575      +31     
+ Misses       4071     4069       -2     
- Partials      536      540       +4     

Flag	Coverage Δ
backend	74.16% <93.10%> (+0.20%)	⬆️
frontend	32.47% <45.83%> (+0.08%)	⬆️

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

Impacted Files	Coverage Δ
src/components/Buttons/FlagButton.tsx	41.17% <ø> (ø)
...Goal/MergeDupStep/DragDropComponents/DragSense.tsx	0.00% <ø> (ø)
...pGoal/MergeDupStep/DragDropComponents/DropWord.tsx	0.00% <0.00%> (ø)
.../MergeDupStep/DragDropComponents/MergeDragDrop.tsx	0.00% <0.00%> (ø)
...rgeDupStep/DragDropComponents/SidebarDragSense.tsx	0.00% <ø> (ø)
...als/MergeDupGoal/MergeDupStep/SenseCardContent.tsx	0.00% <0.00%> (ø)
...eviewEntriesComponent/CellComponents/SenseCell.tsx	0.00% <0.00%> (ø)
...wEntries/ReviewEntriesComponent/tests/MockWords.ts	100.00% <ø> (ø)
src/types/word.ts	90.47% <ø> (ø)
...viewEntriesComponent/CellComponents/DeleteCell.tsx	46.15% <25.00%> (+4.48%)	⬆️
... and 10 more

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[imnasnainaec]

Still to-do:

• tests & comments
• user feedback in MergeDup to explain why protected words/senses are preventing some action
• prevent final sense from leaving protected word
• determine whether protected senses should be allowed to leave unprotected words (for now, no)

[jasonleenaylor]

Reviewed 2 of 7 files at r1, 21 of 30 files at r2, 10 of 11 files at r3, all commit messages.
Reviewable status: 31 of 33 files reviewed, 1 unresolved discussion (waiting on @imnasnainaec)

---

src/components/DataEntry/DataEntryComponent.tsx line 61 at r3 (raw file):

    const senses = currentWord.senses.filter((s) =>
      [Status.Active, Status.Protected, undefined].includes(s.accessibility)
    );

We should either retain the comment about why undefined is allowed, or remove it if we can prove it is now obsolete.

Code quote:

    const senses = currentWord.senses.filter((s) =>
      [Status.Active, Status.Protected, undefined].includes(s.accessibility)
    );

---

src/components/DataEntry/tests/DataEntryComponent.test.tsx line 39 at r3 (raw file):

        {
          ...mockWord,
          senses: [{ ...newSense(), accessibility: Status.Protected }],

I can't tell from just this code if the test is valid. Does newSense contain an Active sense, and does that impact this logic?
It is probably fine, just checking.

Code quote:

senses: [{ ...newSense(), accessibility: Status.Protected }],

---

src/goals/MergeDupGoal/MergeDupStep/SenseCardContent.tsx line 134 at r3 (raw file):

    ),
  ];
  // If props.languages is undefined, this is a sidebar sense and needs no warning.

I'm not sure if I like relying on this to determine if it is a sidebar sense. It seems fragile. Maybe we should have a property to specifically indicate the sidebar senses?

Code quote:

  // If props.languages is undefined, this is a sidebar sense and needs no warning.
  const protectedWarning =
    props.languages !== undefined &&

[imnasnainaec]

Reviewable status: 27 of 33 files reviewed, all discussions resolved (waiting on @imnasnainaec and @jasonleenaylor)

---

src/components/DataEntry/DataEntryComponent.tsx line 61 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

We should either retain the comment about why undefined is allowed, or remove it if we can prove it is now obsolete.

Restored.

---

src/components/DataEntry/tests/DataEntryComponent.test.tsx line 39 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

I can't tell from just this code if the test is valid. Does newSense contain an Active sense, and does that impact this logic?
It is probably fine, just checking.

newSense() does have accessibility=Status.Active, which is explicitly overridden here.

---

src/goals/MergeDupGoal/MergeDupStep/SenseCardContent.tsx line 134 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

I'm not sure if I like relying on this to determine if it is a sidebar sense. It seems fragile. Maybe we should have a property to specifically indicate the sidebar senses?

Silly of me. Fixed.

[jasonleenaylor]

Reviewed 1 of 30 files at r2, 1 of 11 files at r3, 4 of 4 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

[jasonleenaylor]

Backend/Helper/LiftHelper.cs line 14 at r4 (raw file):

        {
            return entry.Annotations.Count > 0 || entry.Etymologies.Count > 0 || entry.Fields.Count > 0 ||
                entry.Pronunciations.Count > 0 || entry.Relations.Count > 0 || entry.Traits.Count > 0 ||

Ok, this will need some work. I just used the Collect Words tool in FLEx to generate a couple of entries.
and <trait name="morph-type" value="stem" /> was present on the entries.

Code quote:

entry.Traits.Count > 0 ||

[jasonleenaylor]

Backend/Helper/LiftHelper.cs line 25 at r4 (raw file):

            return sense.Examples.Count > 0 || sense.Fields.Count > 0 || sense.GramInfo != null ||
                sense.Illustrations.Count > 0 || sense.Relations.Count > 0 || sense.Reversals.Count > 0 ||
                sense.Subsenses.Count > 0 || (sense.Traits.Find(t => !t.Name.StartsWith("semantic-domain")) != null);

Could you use Sense.Traits.Any(t => !t.Name.StartsWith("semantic-domain")?

Code quote:

(sense.Traits.Find(t => !t.Name.StartsWith("semantic-domain")) != null

[jasonleenaylor]

Backend/Helper/LiftHelper.cs line 13 at r4 (raw file):

        public static bool IsProtected(LiftEntry entry)
        {
            return entry.Annotations.Count > 0 || entry.Etymologies.Count > 0 || entry.Fields.Count > 0 ||

Ok, looking at some lift files I found and tags which we will also want to protect.

[jasonleenaylor]

Backend/Helper/LiftHelper.cs line 13 at r4 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Ok, looking at some lift files I found and tags which we will also want to protect.

Hmm, let's try that again. I found <illustration> and <reversal> tags that we will also want to protect.

[jasonleenaylor]

Backend/Helper/LiftHelper.cs line 13 at r4 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Hmm, let's try that again. I found <illustration> and <reversal> tags that we will also want to protect.

Note with a "type" other than empty probably should also cause protection.

[imnasnainaec]

Reviewable status: 32 of 33 files reviewed, 2 unresolved discussions (waiting on @jasonleenaylor)

---

Backend/Helper/LiftHelper.cs line 13 at r4 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Note with a "type" other than empty probably should also cause protection.

Note protections added.

---

Backend/Helper/LiftHelper.cs line 14 at r4 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Ok, this will need some work. I just used the Collect Words tool in FLEx to generate a couple of entries.
and <trait name="morph-type" value="stem" /> was present on the entries.

"stem" Trait protection removed.

---

Backend/Helper/LiftHelper.cs line 25 at r4 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Could you use Sense.Traits.Any(t => !t.Name.StartsWith("semantic-domain")?

done

[jasonleenaylor]

Backend/Helper/LiftHelper.cs line 15 at r5 (raw file):

        {
            return entry.Annotations.Count > 0 || entry.Etymologies.Count > 0 || entry.Fields.Count > 0 ||
                (entry.Notes.Count == 1 && String.IsNullOrEmpty(entry.Notes.First().Type)) ||

I think you have the logic inverted on the nullorempty check here.

[imnasnainaec]

Reviewable status: 32 of 33 files reviewed, 2 unresolved discussions (waiting on @jasonleenaylor)

---

Backend/Helper/LiftHelper.cs line 15 at r5 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

I think you have the logic inverted on the nullorempty check here.

flipped

[jasonleenaylor]

Reviewed 1 of 1 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)