fix: disappearing sorts (#4691)

* chore: copy the field type

* chore: don't store field type in Sort

* fix: sorts disappearing
This commit is contained in:
Richard Shiue
2024-02-21 20:01:57 +08:00
committed by GitHub
parent 58aa73b5be
commit 34fb1bcfa4
20 changed files with 43 additions and 61 deletions

View File

@ -289,7 +289,7 @@ class FieldController {
final fieldInfo = _findFieldInfo( final fieldInfo = _findFieldInfo(
fieldInfos: fieldInfos, fieldInfos: fieldInfos,
fieldId: newSortPB.sort.fieldId, fieldId: newSortPB.sort.fieldId,
fieldType: newSortPB.sort.fieldType, fieldType: null,
); );
if (fieldInfo != null) { if (fieldInfo != null) {
@ -318,7 +318,7 @@ class FieldController {
final fieldInfo = _findFieldInfo( final fieldInfo = _findFieldInfo(
fieldInfos: fieldInfos, fieldInfos: fieldInfos,
fieldId: updatedSort.fieldId, fieldId: updatedSort.fieldId,
fieldType: updatedSort.fieldType, fieldType: null,
); );
if (fieldInfo != null) { if (fieldInfo != null) {
@ -688,7 +688,7 @@ class FieldController {
final fieldInfo = _findFieldInfo( final fieldInfo = _findFieldInfo(
fieldInfos: fieldInfos, fieldInfos: fieldInfos,
fieldId: sortPB.fieldId, fieldId: sortPB.fieldId,
fieldType: sortPB.fieldType, fieldType: null,
); );
return fieldInfo != null return fieldInfo != null
? SortInfo(sortPB: sortPB, fieldInfo: fieldInfo) ? SortInfo(sortPB: sortPB, fieldInfo: fieldInfo)
@ -847,9 +847,11 @@ class RowCacheDependenciesImpl extends RowFieldsDelegate with RowLifeCycle {
FieldInfo? _findFieldInfo({ FieldInfo? _findFieldInfo({
required List<FieldInfo> fieldInfos, required List<FieldInfo> fieldInfos,
required String fieldId, required String fieldId,
required FieldType fieldType, required FieldType? fieldType,
}) { }) {
return fieldInfos.firstWhereOrNull( return fieldInfos.firstWhereOrNull(
(element) => element.id == fieldId && element.fieldType == fieldType, (element) =>
element.id == fieldId &&
(fieldType == null || element.fieldType == fieldType),
); );
} }

View File

@ -31,7 +31,6 @@ class SortBackendService {
}) { }) {
final insertSortPayload = UpdateSortPayloadPB.create() final insertSortPayload = UpdateSortPayloadPB.create()
..fieldId = fieldId ..fieldId = fieldId
..fieldType = fieldType
..viewId = viewId ..viewId = viewId
..condition = condition ..condition = condition
..sortId = sortId; ..sortId = sortId;
@ -57,7 +56,6 @@ class SortBackendService {
}) { }) {
final insertSortPayload = UpdateSortPayloadPB.create() final insertSortPayload = UpdateSortPayloadPB.create()
..fieldId = fieldId ..fieldId = fieldId
..fieldType = fieldType
..viewId = viewId ..viewId = viewId
..condition = condition; ..condition = condition;

View File

@ -24,7 +24,6 @@ export async function insertSort(viewId: string, sort: Omit<Sort, 'id'>): Promis
update_sort: { update_sort: {
view_id: viewId, view_id: viewId,
field_id: sort.fieldId, field_id: sort.fieldId,
field_type: sort.fieldType,
condition: sort.condition, condition: sort.condition,
}, },
}); });
@ -41,7 +40,6 @@ export async function updateSort(viewId: string, sort: Sort): Promise<void> {
view_id: viewId, view_id: viewId,
sort_id: sort.id, sort_id: sort.id,
field_id: sort.fieldId, field_id: sort.fieldId,
field_type: sort.fieldType,
condition: sort.condition, condition: sort.condition,
}, },
}); });

View File

@ -1,9 +1,8 @@
import { FieldType, SortConditionPB, SortPB } from '@/services/backend'; import { SortConditionPB, SortPB } from '@/services/backend';
export interface Sort { export interface Sort {
id: string; id: string;
fieldId: string; fieldId: string;
fieldType: FieldType;
condition: SortConditionPB; condition: SortConditionPB;
} }
@ -11,7 +10,6 @@ export function pbToSort(pb: SortPB): Sort {
return { return {
id: pb.id, id: pb.id,
fieldId: pb.field_id, fieldId: pb.field_id,
fieldType: pb.field_type,
condition: pb.condition, condition: pb.condition,
}; };
} }

View File

@ -18,7 +18,6 @@ const SortFieldsMenu: FC<
async (event: MouseEvent, field: Field) => { async (event: MouseEvent, field: Field) => {
await sortService.insertSort(viewId, { await sortService.insertSort(viewId, {
fieldId: field.id, fieldId: field.id,
fieldType: field.type,
condition: SortConditionPB.Ascending, condition: SortConditionPB.Ascending,
}); });
props.onClose?.({}, 'backdropClick'); props.onClose?.({}, 'backdropClick');

View File

@ -21,7 +21,6 @@ export const SortItem: FC<SortItemProps> = ({ className, sort }) => {
void sortService.updateSort(viewId, { void sortService.updateSort(viewId, {
...sort, ...sort,
fieldId: field.id, fieldId: field.id,
fieldType: field.type,
}); });
} }
}, },

View File

@ -46,7 +46,7 @@ impl FieldPB {
let field_type = field.field_type.into(); let field_type = field.field_type.into();
let type_option = field let type_option = field
.get_any_type_option(field_type) .get_any_type_option(field_type)
.unwrap_or_else(|| default_type_option_data_from_type(&field_type)); .unwrap_or_else(|| default_type_option_data_from_type(field_type));
Self { Self {
id: field.id, id: field.id,
name: field.name, name: field.name,

View File

@ -1,7 +1,6 @@
use flowy_derive::{ProtoBuf, ProtoBuf_Enum}; use flowy_derive::{ProtoBuf, ProtoBuf_Enum};
use validator::Validate; use validator::Validate;
use crate::entities::FieldType;
use crate::services::sort::{Sort, SortCondition}; use crate::services::sort::{Sort, SortCondition};
#[derive(Eq, PartialEq, ProtoBuf, Debug, Default, Clone)] #[derive(Eq, PartialEq, ProtoBuf, Debug, Default, Clone)]
@ -13,9 +12,6 @@ pub struct SortPB {
pub field_id: String, pub field_id: String,
#[pb(index = 3)] #[pb(index = 3)]
pub field_type: FieldType,
#[pb(index = 4)]
pub condition: SortConditionPB, pub condition: SortConditionPB,
} }
@ -24,7 +20,6 @@ impl std::convert::From<&Sort> for SortPB {
Self { Self {
id: sort.id.clone(), id: sort.id.clone(),
field_id: sort.field_id.clone(), field_id: sort.field_id.clone(),
field_type: sort.field_type,
condition: sort.condition.into(), condition: sort.condition.into(),
} }
} }
@ -35,7 +30,6 @@ impl std::convert::From<Sort> for SortPB {
Self { Self {
id: sort.id, id: sort.id,
field_id: sort.field_id, field_id: sort.field_id,
field_type: sort.field_type,
condition: sort.condition.into(), condition: sort.condition.into(),
} }
} }
@ -109,15 +103,12 @@ pub struct UpdateSortPayloadPB {
#[validate(custom = "lib_infra::validator_fn::required_not_empty_str")] #[validate(custom = "lib_infra::validator_fn::required_not_empty_str")]
pub field_id: String, pub field_id: String,
#[pb(index = 3)]
pub field_type: FieldType,
/// Create a new sort if the sort_id is None /// Create a new sort if the sort_id is None
#[pb(index = 4, one_of)] #[pb(index = 3, one_of)]
#[validate(custom = "super::utils::validate_sort_id")] #[validate(custom = "super::utils::validate_sort_id")]
pub sort_id: Option<String>, pub sort_id: Option<String>,
#[pb(index = 5)] #[pb(index = 4)]
pub condition: SortConditionPB, pub condition: SortConditionPB,
} }

View File

@ -254,7 +254,7 @@ pub(crate) async fn switch_to_field_handler(
let database_editor = manager.get_database_with_view_id(&params.view_id).await?; let database_editor = manager.get_database_with_view_id(&params.view_id).await?;
let old_field = database_editor.get_field(&params.field_id); let old_field = database_editor.get_field(&params.field_id);
database_editor database_editor
.switch_to_field_type(&params.field_id, &params.field_type) .switch_to_field_type(&params.field_id, params.field_type)
.await?; .await?;
if let Some(new_type_option) = database_editor if let Some(new_type_option) = database_editor

View File

@ -373,7 +373,7 @@ impl DatabaseEditor {
pub async fn switch_to_field_type( pub async fn switch_to_field_type(
&self, &self,
field_id: &str, field_id: &str,
new_field_type: &FieldType, new_field_type: FieldType,
) -> FlowyResult<()> { ) -> FlowyResult<()> {
let field = self.database.lock().fields.get_field(field_id); let field = self.database.lock().fields.get_field(field_id);
match field { match field {
@ -531,7 +531,7 @@ impl DatabaseEditor {
let type_option_data = params let type_option_data = params
.type_option_data .type_option_data
.and_then(|data| type_option_data_from_pb(data, &params.field_type).ok()) .and_then(|data| type_option_data_from_pb(data, &params.field_type).ok())
.unwrap_or(default_type_option_data_from_type(&params.field_type)); .unwrap_or(default_type_option_data_from_type(params.field_type));
let (index, field) = self.database.lock().create_field_with_mut( let (index, field) = self.database.lock().create_field_with_mut(
&params.view_id, &params.view_id,

View File

@ -491,7 +491,6 @@ impl DatabaseViewEditor {
let sort = Sort { let sort = Sort {
id: sort_id, id: sort_id,
field_id: params.field_id.clone(), field_id: params.field_id.clone(),
field_type: params.field_type,
condition: params.condition.into(), condition: params.condition.into(),
}; };
@ -791,10 +790,16 @@ impl DatabaseViewEditor {
.await; .await;
} }
pub async fn v_did_update_field_type(&self, field_id: &str, new_field_type: &FieldType) { pub async fn v_did_update_field_type(&self, field_id: &str, new_field_type: FieldType) {
self
.sort_controller
.read()
.await
.did_update_field_type()
.await;
self self
.calculations_controller .calculations_controller
.did_receive_field_type_changed(field_id.to_owned(), new_field_type.to_owned()) .did_receive_field_type_changed(field_id.to_owned(), new_field_type)
.await; .await;
} }

View File

@ -18,7 +18,7 @@ impl FieldBuilder {
} }
pub fn from_field_type(field_type: FieldType) -> Self { pub fn from_field_type(field_type: FieldType) -> Self {
let type_option_data = default_type_option_data_from_type(&field_type); let type_option_data = default_type_option_data_from_type(field_type);
Self::new(field_type, type_option_data) Self::new(field_type, type_option_data)
} }

View File

@ -264,13 +264,13 @@ pub fn type_option_to_pb(type_option: TypeOptionData, field_type: &FieldType) ->
} }
} }
pub fn default_type_option_data_from_type(field_type: &FieldType) -> TypeOptionData { pub fn default_type_option_data_from_type(field_type: FieldType) -> TypeOptionData {
match field_type { match field_type {
FieldType::RichText => RichTextTypeOption::default().into(), FieldType::RichText => RichTextTypeOption::default().into(),
FieldType::Number => NumberTypeOption::default().into(), FieldType::Number => NumberTypeOption::default().into(),
FieldType::DateTime => DateTypeOption::default().into(), FieldType::DateTime => DateTypeOption::default().into(),
FieldType::LastEditedTime | FieldType::CreatedTime => TimestampTypeOption { FieldType::LastEditedTime | FieldType::CreatedTime => TimestampTypeOption {
field_type: *field_type, field_type,
include_time: true, include_time: true,
..Default::default() ..Default::default()
} }

View File

@ -496,7 +496,7 @@ impl<'a> TypeOptionCellExt<'a> {
pub fn transform_type_option( pub fn transform_type_option(
type_option_data: &TypeOptionData, type_option_data: &TypeOptionData,
new_field_type: &FieldType, new_field_type: FieldType,
old_type_option_data: Option<TypeOptionData>, old_type_option_data: Option<TypeOptionData>,
old_field_type: FieldType, old_field_type: FieldType,
) -> TypeOptionData { ) -> TypeOptionData {
@ -538,7 +538,7 @@ where
} }
fn get_type_option_transform_handler( fn get_type_option_transform_handler(
type_option_data: &TypeOptionData, type_option_data: &TypeOptionData,
field_type: &FieldType, field_type: FieldType,
) -> Box<dyn TypeOptionTransformHandler> { ) -> Box<dyn TypeOptionTransformHandler> {
let type_option_data = type_option_data.clone(); let type_option_data = type_option_data.clone();
match field_type { match field_type {

View File

@ -82,7 +82,7 @@ where
let field_type = FieldType::from(grouping_field.field_type); let field_type = FieldType::from(grouping_field.field_type);
let type_option = grouping_field let type_option = grouping_field
.get_type_option::<T>(&field_type) .get_type_option::<T>(&field_type)
.unwrap_or_else(|| T::from(default_type_option_data_from_type(&field_type))); .unwrap_or_else(|| T::from(default_type_option_data_from_type(field_type)));
// TODO(nathan): remove block_on // TODO(nathan): remove block_on
let generated_groups = block_on(G::build(grouping_field, &configuration, &type_option)); let generated_groups = block_on(G::build(grouping_field, &configuration, &type_option));

View File

@ -143,7 +143,7 @@ fn database_from_fields_and_rows(
fn default_field(field_str: String, is_primary: bool) -> Field { fn default_field(field_str: String, is_primary: bool) -> Field {
let field_type = FieldType::RichText; let field_type = FieldType::RichText;
let type_option_data = default_type_option_data_from_type(&field_type); let type_option_data = default_type_option_data_from_type(field_type);
Field::new(gen_field_id(), field_str, field_type.into(), is_primary) Field::new(gen_field_id(), field_str, field_type.into(), is_primary)
.with_type_option_data(field_type, type_option_data) .with_type_option_data(field_type, type_option_data)
} }

View File

@ -105,6 +105,14 @@ impl SortController {
} }
} }
pub async fn did_update_field_type(&self) {
if !self.sorts.is_empty() {
self
.gen_task(SortEvent::SortDidChanged, QualityOfService::Background)
.await;
}
}
// #[tracing::instrument(name = "process_sort_task", level = "trace", skip_all, err)] // #[tracing::instrument(name = "process_sort_task", level = "trace", skip_all, err)]
pub async fn process(&mut self, predicate: &str) -> FlowyResult<()> { pub async fn process(&mut self, predicate: &str) -> FlowyResult<()> {
let event_type = SortEvent::from_str(predicate).unwrap(); let event_type = SortEvent::from_str(predicate).unwrap();
@ -285,13 +293,13 @@ fn cmp_row(
fields: &[Arc<Field>], fields: &[Arc<Field>],
cell_data_cache: &CellCache, cell_data_cache: &CellCache,
) -> Ordering { ) -> Ordering {
let field_type = sort.field_type;
match fields match fields
.iter() .iter()
.find(|field_rev| field_rev.id == sort.field_id) .find(|field_rev| field_rev.id == sort.field_id)
{ {
None => default_order(), None => default_order(),
Some(field_rev) => { Some(field_rev) => {
let field_type = field_rev.field_type.into();
let timestamp_cells = match field_type { let timestamp_cells = match field_type {
FieldType::LastEditedTime | FieldType::CreatedTime => { FieldType::LastEditedTime | FieldType::CreatedTime => {
let (left_cell, right_cell) = if field_type.is_created_time() { let (left_cell, right_cell) = if field_type.is_created_time() {

View File

@ -5,31 +5,23 @@ use collab::core::any_map::AnyMapExtension;
use collab_database::rows::RowId; use collab_database::rows::RowId;
use collab_database::views::{SortMap, SortMapBuilder}; use collab_database::views::{SortMap, SortMapBuilder};
use crate::entities::FieldType;
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct Sort { pub struct Sort {
pub id: String, pub id: String,
pub field_id: String, pub field_id: String,
pub field_type: FieldType,
pub condition: SortCondition, pub condition: SortCondition,
} }
const SORT_ID: &str = "id"; const SORT_ID: &str = "id";
const FIELD_ID: &str = "field_id"; const FIELD_ID: &str = "field_id";
const FIELD_TYPE: &str = "ty";
const SORT_CONDITION: &str = "condition"; const SORT_CONDITION: &str = "condition";
impl TryFrom<SortMap> for Sort { impl TryFrom<SortMap> for Sort {
type Error = anyhow::Error; type Error = anyhow::Error;
fn try_from(value: SortMap) -> Result<Self, Self::Error> { fn try_from(value: SortMap) -> Result<Self, Self::Error> {
match ( match (value.get_str_value(SORT_ID), value.get_str_value(FIELD_ID)) {
value.get_str_value(SORT_ID), (Some(id), Some(field_id)) => {
value.get_str_value(FIELD_ID),
value.get_i64_value(FIELD_TYPE).map(FieldType::from),
) {
(Some(id), Some(field_id), Some(field_type)) => {
let condition = value let condition = value
.get_i64_value(SORT_CONDITION) .get_i64_value(SORT_CONDITION)
.map(SortCondition::from) .map(SortCondition::from)
@ -37,7 +29,6 @@ impl TryFrom<SortMap> for Sort {
Ok(Self { Ok(Self {
id, id,
field_id, field_id,
field_type,
condition, condition,
}) })
}, },
@ -53,15 +44,15 @@ impl From<Sort> for SortMap {
SortMapBuilder::new() SortMapBuilder::new()
.insert_str_value(SORT_ID, data.id) .insert_str_value(SORT_ID, data.id)
.insert_str_value(FIELD_ID, data.field_id) .insert_str_value(FIELD_ID, data.field_id)
.insert_i64_value(FIELD_TYPE, data.field_type.into())
.insert_i64_value(SORT_CONDITION, data.condition.value()) .insert_i64_value(SORT_CONDITION, data.condition.value())
.build() .build()
} }
} }
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug, Default)]
#[repr(u8)] #[repr(u8)]
pub enum SortCondition { pub enum SortCondition {
#[default]
Ascending = 0, Ascending = 0,
Descending = 1, Descending = 1,
} }
@ -82,12 +73,6 @@ impl SortCondition {
} }
} }
impl Default for SortCondition {
fn default() -> Self {
Self::Ascending
}
}
impl From<i64> for SortCondition { impl From<i64> for SortCondition {
fn from(value: i64) -> Self { fn from(value: i64) -> Self {
match value { match value {

View File

@ -87,7 +87,7 @@ impl DatabaseFieldTest {
// //
self self
.editor .editor
.switch_to_field_type(&field_id, &new_field_type) .switch_to_field_type(&field_id, new_field_type)
.await .await
.unwrap(); .unwrap();
}, },

View File

@ -79,7 +79,6 @@ impl DatabaseSortTest {
view_id: self.view_id.clone(), view_id: self.view_id.clone(),
field_id: field.id.clone(), field_id: field.id.clone(),
sort_id: None, sort_id: None,
field_type: FieldType::from(field.field_type),
condition: condition.into(), condition: condition.into(),
}; };
let _ = self.editor.create_or_update_sort(params).await.unwrap(); let _ = self.editor.create_or_update_sort(params).await.unwrap();