refactor: migrate externalSettings to new table key structure

[abdimo101]

Description

This PR updates UserSetting externalSettings to new structure:
{
externalSettings: {
fe_dataset_table_columns:{...},
fe_dataset_table_conditions:{...},
fe_dataset_table_filters:{...},
fe_proposal_table_columns:{...},
fe_proposal_table_filters:{...},
fe_sample_table_columns:{...},
fe_sample_table_conditions:{...},
fe_instrument_table_columns:{...},
fe_file_table_columns:{...},
}
}

Motivation

Fixes

• Bug fixed (#X)

Changes:

• changes made

Tests included

• Included for each change/fix?
• Passing?

Documentation

• swagger documentation updated (required for API changes)
• official documentation updated

official documentation info

[Junjiequan]

Is default value needed? externalSettings is optional field if I remember correctly

[minottic]

isn't this PR non backward compatible, since it changes the response of the controllers?

[minottic]

sorry, I am not sure I understand this PR.

It renames the externalSettings keys, but they are not defined in any DTO?

And the migration does not do anything apart from dropping keys?

[abdimo101]

@minottic The purpose of this PR is for dropping old keys inside externalSettings, since we are going to use a different structure that makes the keys more descriptive. Check this PR: SciCatProject/frontend#2274

[minottic]

thanks! if this is a renaming, shouldn't the migration move old keys content to new ones?

[abdimo101]

We decided to not keep the old values because it is easier to just drop it and recreate the values in the frontend for example reordering the columns, enabling/disabling the filters etc.

[minottic]

will all users loose their configs then? what does externalSettings control?

also, why not having a DTO for these, if the structure is defined and fixed?

[Junjiequan]

@minottic The externalSettings contains only users' UI preferences, such as customized table column width, order etc.. if no settings are saved, the default settings will be applied and saved to users externalSettings from frontend. So, it's cleaner way to handle these kinds of updates, just wipping out externalSettings and let it overwritten with default values.

Downside, it might supprises users and annoys them temporarily.
Upside, simple, fast and less code.

It'd also be nice to have a version management for external settings in the future. That being said, if you have concerns about user experience due to the change, I think we can also add complete migration script for that

[minottic]

thanks, understood. Shouldn't the migration only move from "filters" to "fe_dataset_table_filters" etc? If so, the migration code will stay easy I believe.

Also, I have a bit the impression this PR is hard to understand because the externalFields does not have a DTO defined, would it make sense to have one?

[Junjiequan]

thanks, understood. Shouldn't the migration only move from "filters" to "fe_dataset_table_filters" etc? If so, the migration code will stay easy I believe.

It's a little bit more complicated than that, the externalSettings structure looks like this:

{
 "columns" :{...},
 "fiters":{...},
 "conditions":{...},
 "tableSettings: {
    "ProposalTable": {
       "columns" :{...},
       "fiters":{...},
       "conditions":{...},
     },
     "...": {...}
   }
}

It's a bit mess, generally I'd just drop the field entirely and let it be overwritten with default, but ofc there's tradeoffs for user experiences.

Also, I have a bit the impression this PR is hard to understand because the externalFields does not have a DTO defined, would it make sense to have one?

Yes, now, we are getting to the point where having a dto would be beneficial. Previously, we didn't have any idea what fields exactly should be stored in the externalSettings except table settings. But it needs a bit more discussion and final decision to be made by project leader I think

[minottic]

It's a little bit more complicated than that, the externalSettings structure looks like this:

ah I see, thanks for the explanation (though I am not sure I understand where fe_dataset_table_filters would live here).

Makes sense to me. I'll defer formal approval to someone more familiar with this logic, but I'm happy for you to move forward