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

#338 - Handle case where same key is in multiple capsules for an agent #339

Closed
wants to merge 2 commits into from

Conversation

bgillock
Copy link
Contributor

@bgillock bgillock commented Feb 4, 2022

For #338, this change will allow for the display of a key in multiple capsules for a single agent. Will also allow for 1) changing the capsule name, 2) changing the number of keys in the capsule, 3) no capsule name entered, and 4) if a count of 0 is entered, the capsule name is forced to be "".

Copy link
Member

@le-jeu le-jeu left a comment

Choose a reason for hiding this comment

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

You need to use the capsule name to change the correct KoH entry.
I suppose that setting a entry to 0 should delete the entry

@bgillock
Copy link
Contributor Author

bgillock commented Feb 5, 2022

The capsule name is passed to the server in the opKeyPromise called before the keyOnHand method. I have confirmed the capsule is removed from the operation on the server when this code is in place. Nulling the capsule name in operation.ts is needed to make the dialog reflect the state of the server (ie. capsule removed for the key).

@le-jeu
Copy link
Member

le-jeu commented Feb 5, 2022

This about the case where we will support multiple capsules per portal right ?
If I have multiple capsules A and B for the same portal, it's not possible to change the value of one of the capsule, since the KoH lookup doesn't check the capsule name. And if the value is set to 0, we need the capsule name to remove the entry from the KoH array.
the KoH shouldn't hold empty entries (and if it does, we need to fix it)

@bgillock
Copy link
Contributor Author

bgillock commented Feb 6, 2022

Not sure this PR is needed anymore as I cannot have more than one capsule for a key using dev. Before the fix in the server this was possible and this PR was designed to support those cases. Seems the ops with multiple capsules per key were purged so as far as I know, the situation does not exist, hence no need for this PR.

@cloudkucooland
Copy link
Member

The server change (removing capID from the database unique key) was to make the clients behave as they did before.

Long term we need to support multiple capsules sanely. The full team will need to discuss the best way of doing this. I think this PR is as good as any place for that conversation to happen.

@bgillock
Copy link
Contributor Author

bgillock commented Feb 6, 2022

Please see issue #330 where some user stories are presented. I believe that would be a better place to elaborate on handling of capsules. This is a PR, with and is tied to the current solution, which obviously is not adequate.

@bgillock bgillock closed this Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants