osclass / Osclass

With Osclass, get your own classifieds site for free. Build your own Osclass installation and start advertising real estate, jobs or whatever you want- in minutes!
http://osclass.org/
648 stars 343 forks source link

UserForm country_select method ignores the country id if only one country exists #1768

Open jairmilanes opened 9 years ago

jairmilanes commented 9 years ago

The UserForm class method "country_select" ignores the country id if only one country exists, in this case it adds a text input instead of a select input, and also add a hidden input being the "countryId" but it sets to a empty string "", passing only the country name to the text input. When the user saves the form he ends up with a country name in the database bet empty country id, wich can cause problems latter. Here is the method as it is now:

    static public function country_select($countries, $user = null) {
            if( count($countries) > 1 ) {
                parent::generic_select('countryId', $countries, 'pk_c_code', 's_name', __('Select a country...'), (isset($user['fk_c_country_code'])) ? $user['fk_c_country_code'] : null);
            } else {
                parent::generic_input_text('country', ( ! empty($user['s_country']) ? $user['s_country'] : @$countries[0]['s_name']));
                //HERE IT SETS TO A EMPTY COUNTRY ID
                parent::generic_input_hidden('countryId', '');
            }
        } 

And how i believe it should be:

    static public function country_select($countries, $user = null) {
            if( count($countries) > 1 ) {
                parent::generic_select('countryId', $countries, 'pk_c_code', 's_name', __('Select a country...'), (isset($user['fk_c_country_code'])) ? $user['fk_c_country_code'] : null);
            } else {
                parent::generic_input_text('country', ( ! empty($user['s_country']) ? $user['s_country'] : @$countries[0]['s_name']));
                // HERE I ADD THE ID TO THE EMPTY IMPUT
                parent::generic_input_hidden('countryId', ( ! empty($user['fk_c_country_code']) ? $user['fk_c_country_code'] : @$countries[0]['pk_c_code']));
            }
        }

Hope it helps!

Best regards.

dev-101 commented 9 years ago

You can simply remove if conditions and set same behaviour for country_select, region_select and city_select (this is what I did).

Now I noticed there is a bug for region_select and city_select, the reason we may never see second condition is because >= 1 is always true.

https://github.com/osclass/Osclass/blob/master/oc-includes/osclass/frm/User.form.class.php#L111 https://github.com/osclass/Osclass/blob/master/oc-includes/osclass/frm/User.form.class.php#L123

edit: now if I think more about it, the condition probably means if there are no cities and regions defined (zero), could it be stated simply as > 0 ?

jairmilanes commented 9 years ago

Great idea, i tested it and worked fine, but would be nice if this fix is added, i´m having a problem where the user can not save his or her profile, I´m validating location information using js, so as the country_code is never used i can not validate if the user selected a country properly.

Best regards.