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

Facilities should have option to sync KDP even if already registered, and if device does not know it's registered #7672

Closed
jonboiser opened this issue Nov 6, 2020 · 4 comments · Fixed by #8753
Assignees
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) P0 - critical Priority: Release blocker or regression TODO: needs clarification Insufficient information to proceed
Milestone

Comments

@jonboiser
Copy link
Contributor

jonboiser commented Nov 6, 2020

Observed behavior

  1. After a Facility is registered to a KDP project, its FacilityDataset.registered value is set to true, which then block the user from registering it again with a different token.
  2. If a facility is not registered to a project, it can only be synced to other peer devices, but not KDP

Expected behavior

  1. After a Facility is registered to a KDP project, the use can then register it again using a different (or same) token.
  2. If a facility is not registered to a project, it can only be synced to other peer devices, and not KDP

User-facing consequences

Errors and logs

Steps to reproduce

Context

@jonboiser jonboiser added the TODO: needs clarification Insufficient information to proceed label Nov 6, 2020
@jonboiser jonboiser added the APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) label Feb 20, 2021
@jamalex jamalex added the P0 - critical Priority: Release blocker or regression label Apr 27, 2021
@jamalex jamalex added this to the 0.14.8 milestone Apr 27, 2021
@jamalex
Copy link
Member

jamalex commented Apr 27, 2021

This is 110% critical for 0.15 (or a 0.14 patch, if we end up doing another). If someone removes a facility from its project on KDP, there is currently no recourse (other than having @jamalex connect in at midnight and manually unregister it so it can be re-registered, as just happened today 🤕 ).

It seems fine to keep the "Register" (maybe as "Register again") tucked in the kebab menu if it's already flagged as registered, but the key thing is that it should not be deactivated.

A corollary feature on KDP that would further help here would be to unset the "registered" flag on a facility when it's removed from its last LearningCommunity. Next time the server syncs to KDP, it would then realized it was no longer registered and re-highlight the "Register" action.

@rtibbles
Copy link
Member

@jonboiser is it deliberate that point 2 is identical in both cases?

@jonboiser
Copy link
Contributor Author

The expected Point 2 should be slightly different, but now I can't remember how 😕

@jamalex
Copy link
Member

jamalex commented Apr 27, 2021

Regarding 2, on my side, I'd like for there to be a way to sync to KDP even if the local Facility "doesn't know" it's registered, though it should not be prominent (e.g. hidden in the kebab menu). The use case is:

  • Device 1 and 2 are colocated, and Facility A is synced from Device 1 to 2, so they now share a Facility.
  • Device 1 and 2 are separated.
  • Device 1 registers Facility A to the project and syncs it to KDP.
  • Device 2 also has Facility A, which is registered from the point of view of KDP, but not yet from the perspective of Device 2.
  • The way Device 2's copy of Facility A will be become aware of being registered is... by syncing to KDP. But it can't, because it doesn't yet know that it's registered. Catch-22.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) P0 - critical Priority: Release blocker or regression TODO: needs clarification Insufficient information to proceed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants