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

GPII-3594: Replace lifecycle manager dynamic session components with standard subcomponents. #727

Merged
merged 6 commits into from Jan 16, 2019

Conversation

Projects
None yet
3 participants
@cindyli
Copy link
Contributor

commented Jan 11, 2019

No description provided.

@cindyli cindyli requested a review from amb26 Jan 11, 2019

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Jan 11, 2019

// Clean up and reinitialize session.model in preparation for the the next key in
fluid.set(userSession, ["options", "gpiiKey"], undefined);
userSession.createTime = undefined;
userSession.applier.change("", null, "DELETE", "SESSIONCLEANUP");

This comment has been minimized.

Copy link
@amb26

amb26 Jan 13, 2019

Member

Capital letters for source name is a bit unidiomatic, probably "SessionCleanup" is ok

@@ -1,59 +0,0 @@
/*!

This comment has been minimized.

Copy link
@amb26

amb26 Jan 13, 2019

Member

Brilliant savings here

// TODO: Make global map of all users of session state
// activeConfiguration: Assigned in contextManager.updateActiveContextName, consumed in UserUpdate
// gpiiKey, solutionsRegistryEntries: Assigned in initialPayload, consumed in UserUpdate
fluid.set(userSession, ["options", "gpiiKey"], gpiiKey);

This comment has been minimized.

Copy link
@amb26

amb26 Jan 13, 2019

Member

You can't modify a component's options like this (soon they will be immutable) - this will have to go into its model.

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2019

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2019

GPII-3594: Use model.gpiiKey to store "restore" key for the restoreSe…
…ssion to match with the way used by the userSession.
@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2019

@cindyli

This comment has been minimized.

Copy link
Contributor Author

commented Jan 14, 2019

Thanks for the comments, @amb26. All addressed and ready for another look.

@@ -51,7 +38,7 @@ var fluid = fluid || require("infusion"),
*/

fluid.defaults("gpii.lifecycleManager.session", {
gradeNames: ["fluid.modelComponent", "gpii.lifecycleManager.sessionIndexer"],
gradeNames: ["fluid.modelComponent"],
model: {
actionResults: {},
originalSettings: {},

This comment has been minimized.

Copy link
@amb26

amb26 Jan 14, 2019

Member

Add some explanation here that we expect the gpiiKey to appear


// Clean up and reinitialize session.model in preparation for the the next key in
userSession.createTime = undefined;
userSession.applier.change("", null, "DELETE", "SessionCleanup");

This comment has been minimized.

Copy link
@amb26

amb26 Jan 14, 2019

Member

Do this in a transaction so that listeners don't receive multiple notifications

@gpii-bot

This comment has been minimized.

Copy link
Collaborator

commented Jan 14, 2019

@cindyli

This comment has been minimized.

Copy link
Contributor Author

commented Jan 14, 2019

@amb26, comments addressed. Ready for another look.

@amb26 amb26 merged commit b4daca6 into GPII:master Jan 16, 2019

1 check passed

default Build finished.
Details

@cindyli cindyli deleted the cindyli:GPII-3594 branch Jan 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.