[FIX] Sync of non existent field throws exception

[goiaba]

• checks if the field is whitelisted for sync
• if a customField, checks its validity before syncing (through Accounts_CustomFields)
• fixes the case when the field is still absent in the user document

• This implementation does not allow dots in the customField name, because the dots would be treated as nested fields

@RocketChat/core

[rodrigok]

Awesome @goiaba

Can you create unit tests for this method? You can see some tests here https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-markdown/tests/client.tests.js

You can run using meteor npm run testunit

[graywolf336]

This implementation does not allow dots in the customField name, because the dots would be treated as nested fields

This has me worried. Does the existing behavior of Rocket.Chat allow dots in the customField name and it be treated as a regular field and not nested ones? If so, then this is a breaking change...

[rodrigok]

@graywolf336 The existing behavior is the same, the dot is allowed, the difference now is that you can access sub values of the LDAP object

[goiaba]

@rodrigok Sure, I'll do it.

@graywolf336 Don't worry! They were already treated as nested fields in customFields (I was just emphasizing).

[goiaba]

@rodrigok I'm in trouble trying to create the unit tests. Following the example you gave me, I'm not able to import the mut in the test file. It complains that "ReferenceError: Logger is not defined". If I also import the Logger from rocketchat-logger/server.js, then it complains that "ReferenceError: EventEmitter is not defined"...

Any tips?

[vinimdocarmo]

@goiaba, I'll try to help you with the unit tests. But any tips from any of the core team members would be great.

[rodrigok]

@goiaba can you fix the conflicts?

[goiaba]

@rodrigok

Yes, I can for sure.

Besides that, I would love get some help on developing the unit tests for this code. The code you told me to use as guide was not enough. Is there anyone that could help me with that?

[vinimdocarmo]

@goiaba Can you show them the problem that we were getting when executing the unit test? That error that occured because of some problem on package rocketchat:streamer.

[goiaba]

Good idea!

@vinimdocarmo oriented me to follow this guide

So I added to packages/rocketchat-ldap/package.js all the packages that I thought rocketchat:ldap could depend upon (based on Package.onUse):

Package.onTest(function(api){
	api.use('rocketchat:ldap');

	api.use('rocketchat:ldapjs');
	api.use('rocketchat:logger');
	api.use('rocketchat:lib');
	api.use('yasaricli:slugify');
	api.use('ecmascript');
	api.use('sha');
	api.use(['ecmascript', 'random', 'practicalmeteor:mocha']);
	api.mainModule('tests/sync.tests.js');
});

Then I tried to run the tests this way:

[10:08:53] bruno@ideapad-Y700:~/CloudStorage/GitHub/Rocket.Chat$ meteor test-packages ./packages/rocketchat-ldap/

But the result was:

[[[[[ Tests ]]]]]                             

=> Started proxy.                             
=> Started MongoDB.                           
=> Errors prevented startup:                                                       
   
   While processing files with ecmascript (for target web.browser):
   packages/rocketchat:streamer/client/client.js:109:2: super() outside of class constructor (109:2)
   
   While processing files with ecmascript (for target os.linux.x86_64):
   packages/rocketchat:streamer/server/server.js:253:3: 'super' outside of function or class (253:3)
   
=> Your application has errors. Waiting for file change.