Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mess): add translation error messages #452

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Pewillia
Copy link
Contributor

@Pewillia Pewillia commented Jun 5, 2019

closes #379

Description

Testing

Documentation

Checklist

  • Commit messages follow the guidelines
  • Release notes have been updated
  • PR targets the correct release version
  • Help files and documentation have been updated

Remember, it is a muffin offence to open a PR with any of the above checklist items incomplete.

Please keep the original issue up to date with the final solution, expected behaviour, and any additional notes for testers


This change is Reviewable

Copy link
Contributor

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to update the release note with you issue

Look at the errors I found in the first file and go through the rest of your PR and try the reduce the amount of errors then I will take another look.

Reviewable status: 0 of 5 files reviewed, 10 unresolved discussions (waiting on @jolevesq and @Pewillia)


src/app/core/common.service.js, line 23 at r1 (raw file):

 * @return {Object} service  common service
 */
function commonService($translate, $rootScope,events, $timeout, constants) {

Space after,


src/app/core/common.service.js, line 112 at r1 (raw file):

    /**
     * Valdiate form after the current language is set to the supplied value.

Typo to validate


src/app/ui/forms/form.service.js, line 424 at r1 (raw file):

        let errorcode = form.error;
        // if value is not a valid number, called by opacity if invalid value
        if ( errorcode === 'number')  {

No space after (


src/app/ui/forms/form.service.js, line 426 at r1 (raw file):

        if ( errorcode === 'number')  {
           mess = $translate.instant('form.map.wkidnuminvalid');
         }    // value is greater than maximum,called by opacity value positive 2 digits

Space after ,


src/app/ui/forms/form.service.js, line 427 at r1 (raw file):

           mess = $translate.instant('form.map.wkidnuminvalid');
         }    // value is greater than maximum,called by opacity value positive 2 digits
        if ( errorcode === 'max')     {

No space after (


src/app/ui/forms/form.service.js, line 429 at r1 (raw file):

        if ( errorcode === 'max')     {
           mess = $translate.instant('form.map.layeropacitymaxerr');
         }  // value is les than minimim,called by opacity, value negative 2 digits

Space after ,


src/app/ui/forms/form.service.js, line 430 at r1 (raw file):

           mess = $translate.instant('form.map.layeropacitymaxerr');
         }  // value is les than minimim,called by opacity, value negative 2 digits
        if ( errorcode === 'min')     {

No space after (


src/app/ui/forms/form.service.js, line 435 at r1 (raw file):

        else if (errorcode === '302')  {
            mess = $translate.instant('form.map.requirederr');
           } // number less than minimum, callee by expand factor,opacty,tolerance

callee ?


src/app/ui/forms/form.service.js, line 436 at r1 (raw file):

            mess = $translate.instant('form.map.requirederr');
           } // number less than minimum, callee by expand factor,opacty,tolerance
        else if ((errorcode === '101') && (message ==='')) {

Space after ===


src/app/ui/forms/form.service.js, line 442 at r1 (raw file):

            mess = $translate.instant('form.map.layeropacitymaxerr');
        }
        else if ( message !=='') {

No space after (, space after ===

@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch 2 times, most recently from 0005284 to 652b319 Compare June 6, 2019 19:38
Copy link
Contributor Author

@Pewillia Pewillia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, 10 unresolved discussions (waiting on @jolevesq)


src/app/core/common.service.js, line 23 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space after,

Done.


src/app/core/common.service.js, line 112 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Typo to validate

Done.


src/app/ui/forms/form.service.js, line 424 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No space after (

Done.


src/app/ui/forms/form.service.js, line 426 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space after ,

Done.


src/app/ui/forms/form.service.js, line 427 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No space after (

Done.


src/app/ui/forms/form.service.js, line 429 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space after ,

Done.


src/app/ui/forms/form.service.js, line 430 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No space after (

Done.


src/app/ui/forms/form.service.js, line 435 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

callee ?

Done.


src/app/ui/forms/form.service.js, line 436 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space after ===

Done.


src/app/ui/forms/form.service.js, line 442 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No space after (, space after ===

Done.

@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch from 652b319 to b84c5e1 Compare August 27, 2019 22:39
@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch from b84c5e1 to 070f903 Compare September 12, 2019 14:49
@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch 2 times, most recently from b77ab3f to bcfb0c2 Compare October 24, 2019 15:30
Copy link
Contributor

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 5 files at r2.
Reviewable status: 1 of 5 files reviewed, 39 unresolved discussions (waiting on @jolevesq and @Pewillia)


src/app/core/common.service.js, line 119 at r2 (raw file):

    function validateForm() {
        // validate form
        $rootScope.$broadcast('schemaFormValidate');

Why do you have this? You do not need this function I think. You can do the same thing as in summary directive

function validateForm() {
        initState();
        $rootScope.$broadcast(events.avValidateForm);
    }

src/app/ui/forms/form.service.js, line 427 at r1 (raw file):

Previously, Pewillia wrote…

Done.

Only one space between ) and {


src/app/ui/forms/form.service.js, line 416 at r2 (raw file):

     * @return {String} mess the updated message
     */
    function setErrorMessage(form, message, variables) {

Is it an optional parameter?


src/app/ui/forms/form.service.js, line 419 at r2 (raw file):

        let mess = $translate.instant(message);

        let errorcode = form.error;

Instead of doing a bunch of if, you may create and array of object like:

const errCode = [{ error: 101, mess: 'my message' }, { ... }]

mess = errCode.filter(item => { item.error === errorcode; })

It should set the error message from your array of objects without the bunch of if.


src/app/ui/forms/form.service.js, line 427 at r2 (raw file):

           mess = $translate.instant('form.map.layeropacitymaxerr');
         }  // value is les than minimim, called by opacity, value negative 2 digits
        if (errorcode === 'min')     {

Only one space between ) and }


src/app/ui/forms/form.service.js, line 432 at r2 (raw file):

        else if (errorcode === '302')  {
            mess = $translate.instant('form.map.requirederr');
           } // number less than minimum, called by expand factor, opacity, tolerance

} alignement


src/app/ui/forms/form.service.js, line 439 at r2 (raw file):

            mess = $translate.instant('form.map.layeropacitymaxerr');
        }
        else if (message !=='') {

Space between = and '


src/app/ui/forms/form.service.js, line 455 at r2 (raw file):

        // replace values in message string if no parameters passed to replace them with
           mess = mess.replace(/{.+?}/, ' ');
           mess = mess.replace(/{.+?}/, ' ');

Same line twice?
We can put all replace one after the other instead of doing it line be line


src/app/ui/forms/map/map.directive.js, line 622 at r2 (raw file):

                                { 'key': 'tileSchemas[].name', 'onChange': debounceService.registerDebounce((model, item) => {
                                    self.formService.updateId(model, $scope, 'tileSchemas');
                                    self.formService.updateLinkValues($scope, [['tileSchemas', 'id'], ['tileSchemas', 'name']], 'tileId'); }, constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 624 at r2 (raw file):

                                    self.formService.updateLinkValues($scope, [['tileSchemas', 'id'], ['tileSchemas', 'name']], 'tileId'); }, constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },
                                {
                                    'key': 'tileSchemas[].extentSetId', 'validationMessage': form => self.formService.setErrorMessage(form, '', ),

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 657 at r2 (raw file):

                                { 'type': 'template', 'template': commonService.addButton('form.map', 'extentfull', 'setExtent', 'av-setfullext-button'), 'setExtent': () => self.formService.setExtent('full', $scope.model.extentSets) },
                                { 'type': 'template', 'template': commonService.addButton('form.map', 'extentmax', 'setExtent', 'av-setmaxext-button'), 'setExtent': () => self.formService.setExtent('maximum', $scope.model.extentSets) },
                                { 'key': 'extentSets[].id', 'htmlClass': 'av-extentset-id', 'onChange': () => debounceService.registerDebounce(self.formService.updateLinkValues(scope, [['extentSets', 'id']], 'extentId'), constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, 'form.map.requirederr', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 668 at r2 (raw file):

                                                btn[apply]('disabled', '');
                                            }
                                        }, constants.debInput, false), 'validationMessage':  form => self.formService.setErrorMessage(form, '', ['viewValue', 'schema.minimum'] ) }

No space between ] and )


src/app/ui/forms/map/map.directive.js, line 687 at r2 (raw file):

                                    { 'key': 'extentSets[].default', 'items': [
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].default.xmin', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 690 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].default.ymin', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 693 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].default.xmax', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 696 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].default.ymax', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 703 at r2 (raw file):

                                    { 'key': 'extentSets[].full', 'items': [
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].full.xmin', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 706 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].full.ymin', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 709 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].full.xmax', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 712 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].full.ymax', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 719 at r2 (raw file):

                                    { 'key': 'extentSets[].maximum', 'items': [
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].maximum.xmin', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 722 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].maximum.ymin', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 725 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].maximum.xmax', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 728 at r2 (raw file):

                                        ] },
                                        { 'type': 'section', 'htmlClass': 'col-xs-3', 'items': [
                                            { 'key': 'extentSets[].maximum.ymax', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) }

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 744 at r2 (raw file):

                        }, 'notitle': true, 'add': $translate.instant('button.add'), 'items': [
                            { 'key': 'lodSets[]', 'htmlClass': `av-lods-array`, 'items': [
                                { 'key': 'lodSets[].id', 'onChange': () => debounceService.registerDebounce(self.formService.updateLinkValues($scope, [['lodSets', 'id']], 'lodId'), constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 790 at r2 (raw file):

                                    self.formService.copyValueToFormIndex(model, item);
                                    self.formService.updateId(model, $scope, 'baseMaps');
                                    self.formService.updateLinkValues($scope, [['baseMaps', 'id'], ['baseMaps', 'name']], 'initBaseId'); }, constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 791 at r2 (raw file):

                                    self.formService.updateId(model, $scope, 'baseMaps');
                                    self.formService.updateLinkValues($scope, [['baseMaps', 'id'], ['baseMaps', 'name']], 'initBaseId'); }, constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },
                                { 'key': 'baseMaps[].description', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 793 at r2 (raw file):

                                { 'key': 'baseMaps[].description', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },
                                { 'key': 'baseMaps[].typeSummary', 'htmlClass': 'av-form-advance hidden' },
                                { 'key': 'baseMaps[].altText', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 808 at r2 (raw file):

                                    // $timeout(() => { events.$broadcast(events.avVersionSet); }, 1000);
                                }, 'items': [
                                    { 'key': 'baseMaps[].layers[].id', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 809 at r2 (raw file):

                                }, 'items': [
                                    { 'key': 'baseMaps[].layers[].id', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },
                                    { 'key': 'baseMaps[].layers[].layerType', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 811 at r2 (raw file):

                                    { 'key': 'baseMaps[].layers[].layerType', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },
                                    { 'key': 'baseMaps[].layers[].url', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },
                                    { 'key': 'baseMaps[].layers[].opacity', 'htmlClass': 'av-opacity-input av-form-advance hidden av-version-dev av-version-dev-hide', 'validationMessage': form => self.formService.setErrorMessage(form, '',['viewValue', 'schema.minimum', 'schema.maximum'] ) }

Space after second ,
This is weird... for html class you have av-version-dev and av-version-hide but it is not in upstream/develop and it should not be on this element. i don't know how it was apply


src/app/ui/forms/map/map.directive.js, line 819 at r2 (raw file):

                                        // To be brought back with version 2.5 since we want the new element altext (v2.4) to be hide with prod version (2.3)
                                        { 'key': 'baseMaps[].attribution.logo', 'items': [
                                            { 'key': 'baseMaps[].attribution.logo.enabled', 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 847 at r2 (raw file):

                                    self.formService.copyValueToFormIndex(model, item);
                                    self.formService.updateId(model, $scope, 'layers', true);
                                    self.formService.updateLinkValues($scope, [['layers', 'id']], 'initLayerId', 'avLayersIdUpdate'); }, constants.debInput, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 855 at r2 (raw file):

                                        $timeout(() => { angular.element(btn).triggerHandler('click'); }, 0);
                                    }
                                }, constants.delayUpdateColumns, false) , 'validationMessage': form => self.formService.setErrorMessage(form, '', )},

No space between ) and ,
There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 858 at r2 (raw file):

                                { 'type': 'help', 'helpvalue': '<div class="help-block">' + $translate.instant('form.map.urlfile') + '<div>', 'condition': 'model.layers[arrayIndex].layerChoice === \'file\'' },
                                { 'key': 'layers[].refreshInterval', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ['viewValue', 'schema.minimum', 'schema.maximum'] ), 'condition': 'model.layers[arrayIndex].layerChoice !== \'file\' && model.layers[arrayIndex].layerChoice !== \'ogcWfs\'' },
                                { 'key': 'layers[].expectedResponseTime', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ),'condition': 'model.layers[arrayIndex].layerChoice !== \'file\' && model.layers[arrayIndex].layerChoice !== \'ogcWfs\'' },

There is no parameters after the second ,
If not needed, should be optional inside the function
Space between , and 'condition'


src/app/ui/forms/map/map.directive.js, line 872 at r2 (raw file):

                                { 'key': 'layers[].customRenderer', 'htmlClass': 'av-form-advance hidden', 'condition': 'model.layers[arrayIndex].layerChoice === \'esriFeature\' || model.layers[arrayIndex].layerChoice === \'file\' || model.layers[arrayIndex].layerChoice === \'ogcWfs\'' },
                                { 'key': 'layers[].xyInAttribs', 'htmlClass': 'av-form-advance hidden', 'condition': 'model.layers[arrayIndex].layerChoice === \'ogcWfs\'' },
                                { 'key': 'layers[].tolerance', 'htmlClass': 'av-form-advance hidden', 'validationMessage': form => self.formService.setErrorMessage(form, '', ['viewValue', 'schema.minimum', 'schema.maximum'] ),'condition': 'model.layers[arrayIndex].layerChoice === \'esriFeature\' || model.layers[arrayIndex].layerChoice === \'file\' || model.layers[arrayIndex].layerChoice === \'esriDynamic\' || model.layers[arrayIndex].layerChoice === \'ogcWfs\'' },

Space between ) and 'condition'


src/app/ui/forms/map/map.directive.js, line 890 at r2 (raw file):

                                                const btn = $(document.activeElement).closest('.av-layerEntry').find('.av-form-setfields button')[0];
                                                $timeout(() => { angular.element(btn).triggerHandler('click'); }, 0);
                                            }, constants.delayUpdateColumns, false), 'validationMessage': form => self.formService.setErrorMessage(form, '', ) },

There is no parameters after the second ,
If not needed, should be optional inside the function


src/app/ui/forms/map/map.directive.js, line 1039 at r2 (raw file):

                        ] },
                        { 'type': 'help', 'htmlClass': 'av-form-advance hidden', 'helpvalue': '<div class="help-block">' + $translate.instant('form.map.expcoldesc') + '<div>' },
                        { 'key': 'components.areaOfInterest', 'title': $translate.instant('form.map.areasofinterest'), 'htmlClass': 'av-accordion-all av-form-advance hidden', 'startEmpty': true,'onChange': () => {

Again, this is weird because this section is not part of map directive anymore... it is inside plugins directive. Have you did your fetch --all before the rebase?


src/app/ui/header/header.directive.js, line 122 at r2 (raw file):

        commonService.setLang(self.language);
        localStorage.setItem('fgpa-lang', self.language);
        // validate form

You do not need to call the validate form from here.

CommonService.setLang will call

function setLang(value) {
        // show splash with language switch event as parameter
        $translate.use(value).then(() => events.$broadcast(events.avShowSplash, events.avSwitchLanguage));
    }

to broadcast the event and all form are listening to this event to start their language switch

You don't have to listen to the event this function is the one who trigger the event. I see there is a problem in the development and your branch with the language validation. When i switch to French, validation is not always triggered. It may be because of the $timeout

Remove the validation part from your PR. I will do a PR to solve this (should one line of code) or I will give you the line to add to your code.

Copy link
Contributor

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add this to your common.service $timeout(() => $rootScope.$broadcast(events.avValidateForm), constants.delaySetVersion);

    function setLang(value) {
        // show splash with language switch event as parameter
        $translate.use(value).then(() => events.$broadcast(events.avShowSplash, events.avSwitchLanguage));
        
        // trigger validation
        $timeout(() => $rootScope.$broadcast(events.avValidateForm), constants.delaySetVersion);
    }

It is a timeout and it is not the best but it works with minimum modification to the code

Reviewable status: 1 of 5 files reviewed, 39 unresolved discussions (waiting on @jolevesq and @Pewillia)

@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch 2 times, most recently from c9adaaa to cb5fbd7 Compare November 7, 2019 16:26
@Pewillia Pewillia requested a review from jolevesq November 7, 2019 19:27
Copy link
Contributor Author

@Pewillia Pewillia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: 1 of 5 files reviewed, 39 unresolved discussions (waiting on @jolevesq)


src/app/core/common.service.js, line 119 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Why do you have this? You do not need this function I think. You can do the same thing as in summary directive

function validateForm() {
        initState();
        $rootScope.$broadcast(events.avValidateForm);
    }

Done.


src/app/ui/forms/form.service.js, line 416 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Is it an optional parameter?

Done.


src/app/ui/forms/form.service.js, line 419 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Instead of doing a bunch of if, you may create and array of object like:

const errCode = [{ error: 101, mess: 'my message' }, { ... }]

mess = errCode.filter(item => { item.error === errorcode; })

It should set the error message from your array of objects without the bunch of if.

Je utilise la table de traduction des messges d'erreur


src/app/ui/forms/form.service.js, line 427 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Only one space between ) and }

Done.


src/app/ui/forms/form.service.js, line 432 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

} alignement

Done.


src/app/ui/forms/form.service.js, line 439 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space between = and '

Done.


src/app/ui/forms/form.service.js, line 455 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Same line twice?
We can put all replace one after the other instead of doing it line be line

Done.


src/app/ui/forms/map/map.directive.js, line 622 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 624 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 657 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 668 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No space between ] and )

Done.


src/app/ui/forms/map/map.directive.js, line 687 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 690 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 693 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 696 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 703 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 706 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 709 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 712 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 719 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 722 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 725 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 728 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 744 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 790 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 791 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 793 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 808 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 809 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 811 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space after second ,
This is weird... for html class you have av-version-dev and av-version-hide but it is not in upstream/develop and it should not be on this element. i don't know how it was apply

Done.


src/app/ui/forms/map/map.directive.js, line 819 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 847 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 855 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No space between ) and ,
There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 858 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function
Space between , and 'condition'

Done.


src/app/ui/forms/map/map.directive.js, line 872 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space between ) and 'condition'

Done.


src/app/ui/forms/map/map.directive.js, line 890 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

There is no parameters after the second ,
If not needed, should be optional inside the function

Done.


src/app/ui/forms/map/map.directive.js, line 1039 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Again, this is weird because this section is not part of map directive anymore... it is inside plugins directive. Have you did your fetch --all before the rebase?

Done.


src/app/ui/header/header.directive.js, line 122 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

You do not need to call the validate form from here.

CommonService.setLang will call

function setLang(value) {
        // show splash with language switch event as parameter
        $translate.use(value).then(() => events.$broadcast(events.avShowSplash, events.avSwitchLanguage));
    }

to broadcast the event and all form are listening to this event to start their language switch

You don't have to listen to the event this function is the one who trigger the event. I see there is a problem in the development and your branch with the language validation. When i switch to French, validation is not always triggered. It may be because of the $timeout

Remove the validation part from your PR. I will do a PR to solve this (should one line of code) or I will give you the line to add to your code.

Done.

Copy link
Contributor

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do a rebase before your next commit

Reviewable status: 1 of 5 files reviewed, 36 unresolved discussions (waiting on @jolevesq and @Pewillia)


src/app/ui/forms/form.service.js, line 416 at r2 (raw file):

Previously, Pewillia wrote…

Done.

Usually you will not set the optional value to undefined. It is an array so it should bu : variables = []


src/app/ui/forms/form.service.js, line 419 at r2 (raw file):

Previously, Pewillia wrote…

Je utilise la table de traduction des messges d'erreur

I do not see the the array like I said...

const errCode = [
{ error: 'number', mess: 'form.map.wkidnuminvalid' },
{ error: 'max', mess: 'form.map.layeropacitymaxerr' },
{ error: 'min', mess: 'form.map.extentdefxminerr' },
{ error: '302', mess: 'form.map.requirederr' },
{ error: '101', mess: 'form.map.extentdefxminerr' },
{ error: '103', mess: 'form.map.layeropacitymaxerr' }]

mess = $translate.instant(errCode.filter(item => { item.error === errorcode; }))


src/app/ui/forms/map/map.directive.js, line 622 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 624 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 657 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, 'form.map.requirederr')


src/app/ui/forms/map/map.directive.js, line 687 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 690 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 693 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 696 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 703 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 706 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 709 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 712 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 719 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 722 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 725 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 728 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 744 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 790 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 791 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 793 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 808 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 809 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 811 at r2 (raw file):

Previously, Pewillia wrote…

Done.

Space after , '',[view''
No space between ] and )


src/app/ui/forms/map/map.directive.js, line 819 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 847 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 855 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 858 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/forms/map/map.directive.js, line 890 at r2 (raw file):

Previously, Pewillia wrote…

Done.

If optional should be ... ormService.setErrorMessage(form, '')


src/app/ui/header/header.directive.js, line 122 at r2 (raw file):

Previously, Pewillia wrote…

Done.

Remove trailling white spaces
Space between ) and }

@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch 2 times, most recently from c7f975b to 87121ab Compare November 21, 2019 15:48
@Pewillia Pewillia force-pushed the 3379-translate-error-message3 branch from 87121ab to 94d9e69 Compare November 21, 2019 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants