Dev: move entity renderers to models

[Integral]

Why

See #1173.

What

Entity renderers for all models except references are moved to member functions.

[codecov[bot]]

Codecov Report

Merging #1265 into master will decrease coverage by <.01%. The diff coverage is 95.55%.

@@            Coverage Diff            @@
##           master   #1265      +/-   ##
=========================================
- Coverage      89%     89%   -0.01%     
=========================================
  Files         400     401       +1     
  Lines        8980    8979       -1     
=========================================
- Hits         7993    7992       -1     
  Misses        987     987

Impacted Files	Coverage Δ
src/article/models/Funder.js	100% <100%> (ø)	:arrow_up:
src/article/models/Keyword.js	100% <100%> (ø)	:arrow_up:
src/article/models/Organisation.js	83.33% <100%> (ø)	:arrow_up:
src/article/shared/entityRenderers.js	83.93% <100%> (-1.24%)	:arrow_down:
src/article/models/Group.js	100% <100%> (ø)	:arrow_up:
src/article/models/Subject.js	100% <100%> (ø)	:arrow_up:
src/article/models/modelHelpers.js	100% <100%> (ø)
src/article/models/RefContrib.js	100% <100%> (ø)	:arrow_up:
src/article/models/Person.js	81.81% <80%> (-18.19%)	:arrow_down:

---

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 53db4f3...e94ba85. Read the comment docs.

[obuchtala]

@Integral we need to find out why this change is decreasing the test coverage.

[obuchtala]

It seems that the toString() methods are never used. @Integral do you have a suggestion? or should we just remove the toString() and add them when we need them?

[obuchtala]

Strange. It must be something more. There are like +84 more missed lines now. I don't see how the toString() could be responsible for this.

[obuchtala]

@Integral It seems that I have looked at the wrong report. Now it is only +4 misses which maps to the toString() usage. Still, if we are not using them, we should remove them.

[obuchtala]

Commented out unused toString() let's bring them in, when we need them.

[Integral]

@oliver---- There is a case where we might need something like that, for instance in multi select input, e.g. dropdown with affiliations (see #1266). If it's the only one place where we do this I think for now it's better to do it right in component to keep consistency in methods across entity models.

[obuchtala]

Just don't want to have dead code from the get go. In case of Organistation we are using it already.

[obuchtala]

An alternative could be to introduce a mixin or super class (RenderableNode) and have the default toString method there.

[Integral]

Yes! But let's wait with mixin until we'll sure about that

On Tue, 30 Apr 2019 at 12:53, Oliver Buchtala notifications@github.com wrote:

An alternative could be to introduce a mixin or super class (RenderableNode) and have the default toString method there.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/substance/texture/pull/1265#issuecomment-487892973, or mute the thread https://github.com/notifications/unsubscribe-auth/AABMN6QEDAAJBRKFX3OS25TPTAJINANCNFSM4HG4O36A .

[obuchtala]

Then we leave it as is. I.e. without dead code.

[Integral]

Absolute! No dead code!

On Tue, 30 Apr 2019 at 12:55, Oliver Buchtala notifications@github.com wrote:

Then we leave it as is. I.e. without dead code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/substance/texture/pull/1265#issuecomment-487893750, or mute the thread https://github.com/notifications/unsubscribe-auth/AABMN6SZPY623S6R2F3GCCTPTAJSZANCNFSM4HG4O36A .