mirror of
				https://github.com/zulip/zulip.git
				synced 2025-10-31 03:53:50 +00:00 
			
		
		
		
	settings_profile_fields: Keep option value constant.
Previously, the value for an option of the select type custom profile field was set as "order - 1". This commit changes it to remain same even when we reorder the options or delete an option. When we add a new option, its value is set as 1 more than largest value already used. This helps is eliminating various bugs in this subsystem, where user's choice is changed unexpectedly when reordering or deleting options. Discussion - https://chat.zulip.org/#narrow/stream/378-api-design/topic/custom.20profile.20fields.20option.20deletion.
This commit is contained in:
		| @@ -66,15 +66,31 @@ function delete_profile_field(e) { | |||||||
|     update_profile_fields_table_element(); |     update_profile_fields_table_element(); | ||||||
| } | } | ||||||
|  |  | ||||||
| function read_select_field_data_from_form(field_elem) { | function read_select_field_data_from_form(field_elem, old_field_data) { | ||||||
|     const field_data = {}; |     const field_data = {}; | ||||||
|     let field_order = 1; |     let field_order = 1; | ||||||
|  |  | ||||||
|  |     const old_option_value_map = new Map(); | ||||||
|  |     if (old_field_data !== undefined) { | ||||||
|  |         for (const [value, choice] of Object.entries(old_field_data)) { | ||||||
|  |             old_option_value_map.set(choice.text, value); | ||||||
|  |         } | ||||||
|  |     } | ||||||
|  |  | ||||||
|     $(field_elem) |     $(field_elem) | ||||||
|         .find("div.choice-row") |         .find("div.choice-row") | ||||||
|         .each(function () { |         .each(function () { | ||||||
|             const text = $(this).find("input")[0].value; |             const text = $(this).find("input")[0].value; | ||||||
|             if (text) { |             if (text) { | ||||||
|                 field_data[field_order - 1] = {text, order: field_order.toString()}; |                 if (old_option_value_map.get(text) !== undefined) { | ||||||
|  |                     // Resetting the data-value in the form is | ||||||
|  |                     // important if the user removed an option string | ||||||
|  |                     // and then added it back again before saving | ||||||
|  |                     // changes. | ||||||
|  |                     $(this).attr("data-value", old_option_value_map.get(text)); | ||||||
|  |                 } | ||||||
|  |                 const value = $(this).attr("data-value"); | ||||||
|  |                 field_data[value] = {text, order: field_order.toString()}; | ||||||
|                 field_order += 1; |                 field_order += 1; | ||||||
|             } |             } | ||||||
|         }); |         }); | ||||||
| @@ -105,8 +121,23 @@ function update_choice_delete_btn($container, display_flag) { | |||||||
|     } |     } | ||||||
| } | } | ||||||
|  |  | ||||||
|  | function get_value_for_new_option(container) { | ||||||
|  |     const $choice_rows = $(container).find(".choice-row"); | ||||||
|  |     if ($choice_rows.length === 0) { | ||||||
|  |         // Value for the first option is 0. | ||||||
|  |         return 0; | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     const existing_option_values = []; | ||||||
|  |     $choice_rows.each(function () { | ||||||
|  |         existing_option_values.push(Number.parseInt($(this).attr("data-value"), 10)); | ||||||
|  |     }); | ||||||
|  |     existing_option_values.sort(); | ||||||
|  |     return existing_option_values[existing_option_values.length - 1] + 1; | ||||||
|  | } | ||||||
|  |  | ||||||
| function create_choice_row(container) { | function create_choice_row(container) { | ||||||
|     const context = {}; |     const context = {value: get_value_for_new_option(container)}; | ||||||
|     const row = render_settings_profile_field_choice(context); |     const row = render_settings_profile_field_choice(context); | ||||||
|     $(container).append(row); |     $(container).append(row); | ||||||
| } | } | ||||||
| @@ -153,11 +184,11 @@ function set_up_create_field_form() { | |||||||
|     } |     } | ||||||
| } | } | ||||||
|  |  | ||||||
| function read_field_data_from_form(field_type_id, field_elem) { | function read_field_data_from_form(field_type_id, field_elem, old_field_data) { | ||||||
|     // Only read field data if we are creating a select field |     // Only read field data if we are creating a select field | ||||||
|     // or external account field. |     // or external account field. | ||||||
|     if (field_type_id === field_types.SELECT.id) { |     if (field_type_id === field_types.SELECT.id) { | ||||||
|         return read_select_field_data_from_form(field_elem); |         return read_select_field_data_from_form(field_elem, old_field_data); | ||||||
|     } else if (field_type_id === field_types.EXTERNAL_ACCOUNT.id) { |     } else if (field_type_id === field_types.EXTERNAL_ACCOUNT.id) { | ||||||
|         return read_external_account_field_data(field_elem); |         return read_external_account_field_data(field_elem); | ||||||
|     } |     } | ||||||
| @@ -231,7 +262,7 @@ export function parse_field_choices_from_field_data(field_data) { | |||||||
|             order: choice.order, |             order: choice.order, | ||||||
|         }); |         }); | ||||||
|     } |     } | ||||||
|  |     choices.sort((a, b) => a.order - b.order); | ||||||
|     return choices; |     return choices; | ||||||
| } | } | ||||||
|  |  | ||||||
| @@ -260,6 +291,7 @@ function set_up_select_field_edit_form(profile_field, field_data) { | |||||||
|         $choice_list.append( |         $choice_list.append( | ||||||
|             render_settings_profile_field_choice({ |             render_settings_profile_field_choice({ | ||||||
|                 text: choice.text, |                 text: choice.text, | ||||||
|  |                 value: choice.value, | ||||||
|             }), |             }), | ||||||
|         ); |         ); | ||||||
|     } |     } | ||||||
| @@ -317,7 +349,11 @@ function open_edit_form(e) { | |||||||
|         data.name = profile_field.$form.find("input[name=name]").val(); |         data.name = profile_field.$form.find("input[name=name]").val(); | ||||||
|         data.hint = profile_field.$form.find("input[name=hint]").val(); |         data.hint = profile_field.$form.find("input[name=hint]").val(); | ||||||
|         data.field_data = JSON.stringify( |         data.field_data = JSON.stringify( | ||||||
|             read_field_data_from_form(Number.parseInt(field.type, 10), profile_field.$form), |             read_field_data_from_form( | ||||||
|  |                 Number.parseInt(field.type, 10), | ||||||
|  |                 profile_field.$form, | ||||||
|  |                 field_data, | ||||||
|  |             ), | ||||||
|         ); |         ); | ||||||
|  |  | ||||||
|         settings_ui.do_settings_change( |         settings_ui.do_settings_change( | ||||||
|   | |||||||
| @@ -1,4 +1,4 @@ | |||||||
| <div class='choice-row movable-profile-field-row'> | <div class='choice-row movable-profile-field-row' data-value="{{value}}"> | ||||||
|     <i class="fa fa-ellipsis-v" aria-hidden="true"></i> |     <i class="fa fa-ellipsis-v" aria-hidden="true"></i> | ||||||
|     <i class="fa fa-ellipsis-v" aria-hidden="true"></i> |     <i class="fa fa-ellipsis-v" aria-hidden="true"></i> | ||||||
|     <input type='text' placeholder='{{t "New option" }}' value="{{ text }}" /> |     <input type='text' placeholder='{{t "New option" }}' value="{{ text }}" /> | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user