[IMPROVE] General federation improvements

[MarcosSpessatto]

Proposed changes (including videos or screenshots)

I know this changed a lot of files, but the main goal for this PR is not to change any behavior, the goals for the PR are:

• Refactor the code;
• Solve any tech debt;
• Simplify and reuse some parts of the code;
• Remove duplicated code;
• Remove all unsafe type castings;
• Solve all Eslint errors and warnings;
• Split too big files;
• Encapsulate the business logic in a better way, avoiding exposing and leaking internal logic to the unintended layers;
• Improve the actual test cases;
• Add more test cases, since a lot of cases were omitted during the release phase;
• Remove unsafe Object.assign statements and prefer to use the class constructor instead;

Issue(s)

Steps to test or reproduce

Further comments

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging 585b2977339c0be01d7c54ca816c14d4c5769795 into 45ee02dcb5d88d4b0ec6b3bb46bb4840c89faf08 - view on LGTM.com

new alerts:

• 2 for Missing await

[lgtm-com[bot]]

This pull request introduces 3 alerts when merging 2027d96a30afd3f986d2b2be01b212e7b9d86cea into 3ca01a819ba0b8f4344eaf3147f998cf63536ed4 - view on LGTM.com

new alerts:

• 2 for Missing await
• 1 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 9 alerts when merging d017ac41e8cc4ee0e643dcec39da3202a8ff6505 into 2ab6aa9ee8d6c55191ff470e20ad002ac88b3289 - view on LGTM.com

new alerts:

• 9 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 9 alerts when merging d29b7da312242e057cf15d29d3cc77a8adabcfe7 into 99892231512651e93f0a8a5b33ae1f2f7243f39c - view on LGTM.com

new alerts:

• 9 for Unused variable, import, function or class

[codecov[bot]]

Codecov Report

Merging #26150 (0fc3293) into develop (0083b0d) will decrease coverage by 0.01%. The diff coverage is 50.00%.

:exclamation: Current head 0fc3293 differs from pull request most recent head 6212aca. Consider uploading reports for the commit 6212aca to get more accurate results

@@             Coverage Diff             @@
##           develop   #26150      +/-   ##
===========================================
- Coverage    38.50%   38.49%   -0.02%     
===========================================
  Files          794      794              
  Lines        19002    19002              
  Branches      1937     1937              
===========================================
- Hits          7317     7315       -2     
- Misses       11395    11396       +1     
- Partials       290      291       +1     

Flag	Coverage Δ
e2e	38.49% <50.00%> (-0.02%)	:arrow_down:

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

[lgtm-com[bot]]

This pull request introduces 9 alerts when merging cd2135d0d0701807138f6558b559a3efaac4df9b into b9f5bcc0233533ce1312e590cce7d3013a6dbca5 - view on LGTM.com

new alerts:

• 9 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 9 alerts when merging b170bfe455e4149419e073b9fc768493d37b4e19 into b9f5bcc0233533ce1312e590cce7d3013a6dbca5 - view on LGTM.com

new alerts:

• 9 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 5 alerts when merging 8516e0da2b3744cedb1e44f5e05a2e89f4784b10 into 1da6c9e73b3877aee5a0f70535d9106c21798e1f - view on LGTM.com

new alerts:

• 5 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 5 alerts when merging 5dfb380a9731bccd957fa33a7d66fef82111f4f9 into 355819e6fbeba67826db4fe1c2bfc26d24db0d61 - view on LGTM.com

new alerts:

• 5 for Unused variable, import, function or class

[lgtm-com[bot]]

This pull request introduces 1 alert when merging ded6e36bb66a21b8b599e48d02e9e34e0dfffb99 into 720566d17d8b127db3ab0ba3dae439a8958f26fd - view on LGTM.com

new alerts:

• 1 for Invocation of non-function

[kodiakhq[bot]]

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

[kodiakhq[bot]]

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

[kodiakhq[bot]]

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

[CLAassistant]

All committers have signed the CLA.

[kodiakhq[bot]]

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

[MarcosSpessatto]

I don't have the knowledge to say it will in fact break both systems, but the isRemote flag is also a generic flag to identify "not local users".. introducing a new flag can cause other issues as well, like we need to make sure to filter both out..

there is at least another place missing an update to consider the new flag, the statistics for federated users: Rocket.Chat/apps/meteor/app/statistics/server/lib/statistics.ts

Line 275 in 8c2c016

statistics.federatedUsers = federationOverviewData.numberOfFederatedUsers; I wonder if there are others as well

I understand your point and I agree with the statistics example you gave. Indeed both flags are being used for the same purpose, but I respectfully disagree we should use the same for either "old federation" and the new version of it, since if we are using both at the same time, changes happening in one feature will affect directly the other. Another example will be, if you are using the old federation and you enable the new one, some unpredictable things could happen since you will start to use data from one federation to another. Maybe @alansikora can help us on this as well.

[alansikora]

It is exactly what @MarcosSpessatto said. We can't.

And the old federation is going to be removed anyway, so we should not bother about it. We will ensure a full cleanup when the time to remove the old federation comes.