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

Reset mapping before loading configuration #1467

Conversation

abdelr
Copy link
Member

@abdelr abdelr commented Feb 1, 2022

Fixes #1459 (Reset mapping before loading configuration)

@abdelr abdelr added the Importer-Redesign Redesign of observed data import label Feb 1, 2022
@abdelr abdelr self-assigned this Feb 1, 2022
@@ -404,6 +404,8 @@ public void LoadConfigurationWithoutImporting()
if (confirmDroppingOfLoadedSheets())
return;

onResetMappingBasedOnCurrentSheet(this, null);
Copy link
Member

Choose a reason for hiding this comment

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

onResetMappingBasedOnCurrentSheet does not use either e nor null. Can I suggest to remove the argument in this mehtod, call in resetMappingBasedOnCurrentSheet and in the event registration, do (o, e)=> eesetMappingBasedOnCurrentSheet()

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right

[Observation]
public void must_reset_format_based_on_current_sheet()
{
(sut as ImporterPresenterForTest).OnResetMappingBasedOnCurrentSheetInvoked.ShouldBeTrue();
Copy link
Member

Choose a reason for hiding this comment

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

neat.

Copy link
Member

@msevestre msevestre left a comment

Choose a reason for hiding this comment

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

suggestion so that we do not call a method that is an event handler with weird aguments

@abdelr abdelr requested a review from msevestre February 2, 2022 07:50
@msevestre msevestre merged commit 21406e2 into develop Feb 2, 2022
@msevestre msevestre deleted the 1459_reset_mapping_based_on_current_sheet_on_load_configuration branch February 2, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Importer-Redesign Redesign of observed data import
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Importer: Confusing behavior when loading the configuration valid for the selected but not for the first sheet
2 participants