Use new sem dom api

[jasonleenaylor]

This change is 

[imnasnainaec]

Reviewed 24 of 58 files at r1, 8 of 13 files at r2, 2 of 26 files at r3.
Reviewable status: 34 of 68 files reviewed, 9 unresolved discussions (waiting on @imnasnainaec and @jasonleenaylor)

---

Backend/Models/SemanticDomain.cs line 26 at r4 (raw file):

        [BsonElement("id")]
        public string Id { get; set; }

remove extra newline

---

Backend/Services/LiftService.cs line 322 at r4 (raw file):

                foreach (var sd in proj.SemanticDomains)
                {
                    WriteRangeElement(liftRangesWriter, sd.Id, Guid.NewGuid().ToString(), sd.Name);

now that sd will have a .guid, we should use that here instead of generating a new, random one

---

src/i18n.ts line 14 at r4 (raw file):

  .use(initReactI18next)
  .init({
    //debug: true, // Uncomment to troubleshoot// detection: options,

for readability, separate distinct pieces with new line before // detection:...

---

src/backend/index.ts line 96 at r4 (raw file):

const userRoleApi = new Api.UserRoleApi(config, BASE_PATH, axiosInstance);
const wordApi = new Api.WordApi(config, BASE_PATH, axiosInstance);
const semanticDomainApi = new Api.SemanticDomainApi(

to maintain alphabetical order, it would be nice for the new semanticDomainApi to go between projectApi and userApi (here and in the sections below).

---

src/components/TreeView/tests/TreeSearch.test.tsx line 7 at r4 (raw file):

import { Key } from "ts-key-enum";

import "tests/mockReactI18next";

since import "tests/mockReactI18next"; has to come before the other internal imports (which is why it wan't sorted with the rest), it'd be nice to add a newline after it (and/or a comment explicitly stating that it has to come before the others)

---

src/components/TreeView/tests/TreeViewActions.test.tsx line 4 at r4 (raw file):

import thunk from "redux-thunk";

import { TreeActionType } from "../TreeViewReduxTypes";

absolute path for imports preferred

---

src/components/TreeView/tests/TreeViewComponent.test.tsx line 8 at r4 (raw file):

import configureMockStore from "redux-mock-store";
import "tests/mockReactI18next";
import thunk from "redux-thunk";

the redux-thunk import should be in the first import section

---

Backend/Contexts/SemanticDomainContext.cs line 28 at r4 (raw file):

// Add all the IOC bits
// ???
// Profit!

comment cleanup

Code quote:

// Get Context and Model connected to DB, Update SemanticDomainService to access the context to provide what the Controller needs
// Add all the IOC bits
// ???
// Profit!

---

Backend/Interfaces/ISemanticDomainContext.cs line 9 at r4 (raw file):

    {
        public IMongoCollection<SemanticDomainTreeNode> SemanticDomains { get; }
        public IMongoCollection<SemanticDomainFull> FullSemanticDomains { get; }

we don't have public on similar lines in the other I*Context.cs files

Code quote:

        public IMongoCollection<SemanticDomainTreeNode> SemanticDomains { get; }
        public IMongoCollection<SemanticDomainFull> FullSemanticDomains { get; }

[jasonleenaylor]

Reviewable status: 34 of 68 files reviewed, 1 unresolved discussion (waiting on @imnasnainaec and @jasonleenaylor)

---

Backend/Services/LiftService.cs line 322 at r4 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

now that sd will have a .guid, we should use that here instead of generating a new, random one

Hmm, good catch. Although these domains can currently only arrive through lift import, and might not have a GUID.

[codecov-commenter]

Codecov Report

Base: 52.04% // Head: 51.41% // Decreases project coverage by -0.62% ⚠️

Coverage data is based on head (09dc770) compared to base (bcd3464).
Patch coverage: 55.27% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1735      +/-   ##
==========================================
- Coverage   52.04%   51.41%   -0.63%     
==========================================
  Files         274      277       +3     
  Lines        8481     8569      +88     
  Branches      616      630      +14     
==========================================
- Hits         4414     4406       -8     
- Misses       3574     3658      +84     
- Partials      493      505      +12     

Flag	Coverage Δ
backend	79.05% <64.66%> (-1.05%)	⬇️
frontend	32.25% <47.09%> (-0.40%)	⬇️

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

Impacted Files	Coverage Δ
Backend/Models/Project.cs	100.00% <ø> (ø)
Backend/Models/Word.cs	99.44% <ø> (-0.04%)	⬇️
Backend/Repositories/SemanticDomainRepository.cs	0.00% <0.00%> (ø)
src/api/api/project-api.ts	13.63% <ø> (+1.26%)	⬆️
src/components/DataEntry/DataEntryComponent.tsx	37.14% <0.00%> (-2.26%)	⬇️
...ents/DataEntry/DataEntryHeader/DataEntryHeader.tsx	50.00% <ø> (ø)
...onents/DataEntry/DataEntryTable/DataEntryTable.tsx	39.28% <ø> (ø)
src/components/DataEntry/index.ts	0.00% <ø> (ø)
src/components/TreeView/TreeViewReducer.ts	69.23% <0.00%> (ø)
...viewEntriesComponent/CellComponents/DeleteCell.tsx	41.66% <0.00%> (ø)
... and 21 more

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[imnasnainaec]

Reviewed 1 of 13 files at r2.
Reviewable status: 35 of 68 files reviewed, 10 unresolved discussions (waiting on @imnasnainaec and @jasonleenaylor)

a discussion (no related file):
Browsing the SemanticDomain tree, there are empty left arrows for domains with no prev, empty right arrows for domains with no next, and an empty up arrow from the head domain. Tracing this back, it appears that when parent, prev, or next is an empty object in the database, it is populated as a no-data SemanticDomain rather than as the null we want. I tried to remove these fields from the database or to replace them with null in the database, but both result in an error.

---

---

Backend/Models/SemanticDomain.cs line 62 at r5 (raw file):

        public override int GetHashCode()
        {
            return HashCode.Combine(Name, Id, Lang);

What about Guid in the HashCode? (Also, not covered in the current Tests.)

---

Backend/Models/SemanticDomain.cs line 114 at r5 (raw file):

        public override int GetHashCode()
        {
            return HashCode.Combine(Name, Id, Description, Questions);

What about Lang and Guid in the HashCode? (Also, not covered in the current Tests.)

---

Backend/Models/SemanticDomain.cs line 147 at r5 (raw file):

        public SemanticDomain? Parent { get; set; }
        [BsonElement("children")]
        public List<SemanticDomain> Children { get; set; }

Should we make Children to be [Required] since it should always exists, even if only an empty set?

---

Backend/Repositories/SemanticDomainRepository.cs line 24 at r5 (raw file):

                filterDef.Eq(x => x.Lang, lang));

            var filterEn = filterDef.Eq("Lang", "en");

Is there a reason to prefer "Lang" and "id" in some places, but x => x.Lang and x => x.Id elsewhere?

Code quote:

                filterDef.Eq("id", id),
                filterDef.Eq(x => x.Lang, lang));

            var filterEn = filterDef.Eq("Lang", "en");
            var filterId = filterDef.Eq("id", "1");

---

Backend/Repositories/SemanticDomainRepository.cs line 28 at r5 (raw file):

            Console.WriteLine("Total in Lang = " + lang + ";id=" + id + ": " + _context.SemanticDomains.CountDocuments(filter: filterEn));
            Console.WriteLine("Total in Lang = " + lang + ";id=" + id + ": " + _context.SemanticDomains.CountDocuments(filter: filterId));

Are these two Console.WriteLines meant to persist?

---

Backend/Repositories/SemanticDomainRepository.cs line 58 at r5 (raw file):

                Console.WriteLine(e.Message);
                Console.WriteLine("****");
                Console.WriteLine(e.StackTrace);

These WriteLines are on the first two functions but not the third.

---

Backend/Services/LiftService.cs line 324 at r5 (raw file):

                   var guid = string.IsNullOrEmpty(sd.Guid) || sd.Guid == Guid.Empty.ToString()
						  ? Guid.NewGuid().ToString()
						  : sd.Guid.ToString();

Isn't sd.Guid already a string?

---

src/i18n.ts line 14 at r4 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

for readability, separate distinct pieces with new line before // detection:...

Oh, I think the newline is being removed by Prettier. Maybe //debug: true, // Uncomment to troubleshoot can be moved to the end of .init({ }).

---

src/backend/index.ts line 408 at r5 (raw file):

  );
  if (response.data) {
    return response.data;

Why is this check necessary only on this function?

[jasonleenaylor]

Reviewable status: 35 of 69 files reviewed, 4 unresolved discussions (waiting on @imnasnainaec and @jasonleenaylor)

---

Backend/Models/SemanticDomain.cs line 147 at r5 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

Should we make Children to be [Required] since it should always exists, even if only an empty set?

I'm not sure if this will gain us anything, I didn't have a chance to fully evaluate it yet.

[imnasnainaec]

Reviewed 3 of 58 files at r1, 2 of 26 files at r3, 1 of 3 files at r4, 1 of 8 files at r5, 1 of 2 files at r6, 1 of 5 files at r7, 1 of 2 files at r8.
Reviewable status: 45 of 69 files reviewed, 2 unresolved discussions (waiting on @imnasnainaec and @jasonleenaylor)

[lgtm-com]

This pull request introduces 1 alert when merging 5e75191 into d73bf4d - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[imnasnainaec]

Reviewed 1 of 13 files at r2, 1 of 17 files at r9.
Reviewable status: 43 of 70 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

[imnasnainaec]

Reviewed 1 of 2 files at r8, 2 of 17 files at r9, 1 of 5 files at r10.
Reviewable status: 46 of 70 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

[jmgrady]

Reviewed 14 of 58 files at r1, 1 of 2 files at r15, all commit messages.
Reviewable status: 48 of 70 files reviewed, all discussions resolved (waiting on @imnasnainaec and @jasonleenaylor)

[jmgrady]

Reviewed 1 of 58 files at r1.
Reviewable status: 48 of 70 files reviewed, all discussions resolved (waiting on @imnasnainaec and @jasonleenaylor)

[jmgrady]

Reviewed 18 of 26 files at r3, 2 of 3 files at r4, 4 of 8 files at r5, 1 of 5 files at r7, 12 of 17 files at r9, 2 of 5 files at r10, 1 of 2 files at r15, 2 of 2 files at r16.
Reviewable status: 67 of 70 files reviewed, all discussions resolved (waiting on @imnasnainaec)

[jmgrady]

Reviewed 2 of 17 files at r9, 1 of 1 files at r12.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jasonleenaylor)