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.

Sign up for GitHub

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

Jump to bottom

refactor: Move indexer to client and add file persistence layer #2348

Merged
merged 46 commits into from
Apr 3, 2020
Merged

refactor: Move indexer to client and add file persistence layer #2348

merged 46 commits into from
Apr 3, 2020

Conversation

lei9444
Copy link
Contributor

@lei9444 lei9444 commented Mar 24, 2020 •
edited
Loading

Description

  1. move indexer from server to client

  2. add file persistence layer to do data synchronizing between client and server
    image

    File persistence will operation the change for every file
So I remove the http request and denounce in file operation action.

Task Item

refs #2103
refs #2247

Screenshots

@lei9444 lei9444 changed the title ref: Move indexer to client and add file persistence layer refactor: Move indexer to client and add file persistence layer Mar 24, 2020
@lei9444 lei9444 marked this pull request as ready for review March 27, 2020 13:38
a-b-r-o-w-n
a-b-r-o-w-n previously requested changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@a-b-r-o-w-n a-b-r-o-w-n left a comment

Choose a reason for hiding this comment

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

This breaks the scenario of initiating the create dialog flow from the from in a BeginDialog action.

@lei9444
Copy link
Contributor Author

lei9444 commented Mar 30, 2020

This breaks the scenario of initiating the create dialog flow from the from in a BeginDialog action.

Hi, @a-b-r-o-w-n I have reverted some code about create dialog, and it works now.

@lei9444
Copy link
Contributor Author

lei9444 commented Apr 1, 2020 •
edited
Loading

@a-b-r-o-w-n @boydc2014 @cwhitten the coverage has increased now
image
If this refactor is Ok, I think next PR I can refactor the undo/redo. It's too expensive to wrapper the actions now. I want to change it to @cwhitten mentioned before #729 (comment)

boydc2014
boydc2014 requested changes Apr 2, 2020
View reviewed changes
Copy link
Contributor

@boydc2014 boydc2014 left a comment

Choose a reason for hiding this comment

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

see above

@boydc2014
Copy link
Contributor

Offline talked with Leilei, to clarify that ideally we don't want actions and reducers to care about or even know about the persistence layer exists. We want to make it totally transparent, so that upper layer will perform on a stable, fast local store.

Since we managed the middle-ware chaining, so the perfect place to do this, is have a middle ware injected after the reducer computed the store update and notify persistence layer.

@lei9444
Copy link
Contributor Author

lei9444 commented Apr 3, 2020

@a-b-r-o-w-n @cwhitten @boydc2014 I have changed the notification mechanism for persistence layer. I wrapped the reducer, so the layer can get the action, stale state and the new state. I can get the processed file content from state and use action type and action.payload.id to find which file is updated. The reducer itself doesn't known if the persistence layer exists.

@cwhitten cwhitten merged commit 8e5a799 into microsoft:master Apr 3, 2020
This was referenced Apr 3, 2020
Closed
Closed
@lei9444 lei9444 deleted the single branch April 14, 2020 08:28
@a-b-r-o-w-n a-b-r-o-w-n mentioned this pull request May 15, 2020
Merged
lei9444 added a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
@lei9444 @a-b-r-o-w-n
@yeze322 @cwhitten
refactor: Move indexer to client and add file persistence layer (micr…
70ad8bf 
…osoft#2348)

* add file persistence layer

* fix some confilct

* support lu publish

* add locale for create

* update the undo/redo

* fix unit test

* handle error

* update the resover

* add args

* remove some notes

* fix create dialog from form

* fix find wrong root dialog

* add error handler

* add locale when search the common lg file

* move file persistence from action to reducer

* fix some conflicts

* remove file persisten midleware

* update the navigate

* fix some conflicts

* change type->kind

* fix conflict

* add unit test for persistence layer

* add some unit test

* add unit tests for lu publish status

* add unit test for file reloatedreducer

* error handling

* wrap the reducer

* refine the persistence

* fix lint

* fix some spelling mistakes

Co-authored-by: Andy Brown <asbrown002@gmail.com>
Co-authored-by: zeye <2295905420@qq.com>
Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-b-r-o-w-n a-b-r-o-w-n a-b-r-o-w-n left review comments

@boydc2014 boydc2014 boydc2014 requested changes

@yeze322 yeze322 yeze322 requested changes

@zhixzhan zhixzhan zhixzhan requested changes

@cwhitten cwhitten cwhitten approved these changes

@corinagum corinagum Awaiting requested review from corinagum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lei9444 @boydc2014 @cwhitten @a-b-r-o-w-n @yeze322 @zhixzhan