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

Setup Wizard - 'Responsibilities of administrator' is shown twice when importing from a device with multiple facilities #12894

Open
pcenov opened this issue Nov 28, 2024 · 15 comments
Assignees
Labels
APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) bug Behavior is wrong or broken DEV: frontend help wanted Open source contributors welcome P2 - normal Priority: Nice to have

Comments

@pcenov
Copy link
Member

pcenov commented Nov 28, 2024

Observed behavior

The 'Responsibilities of administrator' page is shown twice when importing from a device with one or multiple facilities:

responsibilities.mp4

The issue is extant in both 0.17.3 and 0.18.

Expected behavior

The 'Responsibilities of administrator' page should be displayed only once.

Steps to reproduce the issue

  1. Install the build asset from Conditionalize hideContinue on whether we are displaying an error or not #12893 on one device.
  2. Install it on another device.
  3. On the first device setup a device with one or multiple facilities
  4. On the second device go through the 'Import all data from an existing learning facility' flow.

Logs

logs.zip

Usage Details

Kolibri 0.17.3 and 0.18
Ubuntu 22 - Chrome

@pcenov
Copy link
Member Author

pcenov commented Nov 28, 2024

@radinamatic

@radinamatic radinamatic added bug Behavior is wrong or broken P2 - normal Priority: Nice to have APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend labels Dec 2, 2024
@MisRob MisRob added the help wanted Open source contributors welcome label Dec 2, 2024
@MisRob
Copy link
Member

MisRob commented Dec 2, 2024

@Jeevansm25
Copy link

Can I take this issue?

@AlexVelezLl
Copy link
Member

Hi @Jeevansm25! For sure! I will assign this issue to you. Please be aware that this issue is in our setup_wizard plugin. To reproduce this issue you need to have a fresh start of Kolibri so you will need to create new folders and override your KOLIBRI_HOME, or remove your default KOLIBRI_HOME folder each time you finished the setup and want to reproduce it again. To have another Kolibri instance running for the import step you can check MisRob's comment. Let us know if you have any questions.

Jeevansm25 added a commit to Jeevansm25/kolibri that referenced this issue Dec 4, 2024
## Description
This pull request addresses issue learningequality#12894, where the 'Responsibilities of administrator' step is shown twice when importing from a device with multiple facilities in the Kolibri Setup Wizard.

## Problem
The current implementation incorrectly calculates the step count for the import facility flow, leading to a duplicate display of the 'Responsibilities of administrator' page when multiple facilities are present on the device being imported from.

## Solution
The solution involves modifying the `PersonalDataConsentForm` component to:
1. Always return step 4 for import facility scenarios, preventing duplicate steps.
2. Correctly calculate the total number of steps based on the number of facilities.
3. Implement proper error handling for permission issues.

## Changes Made
- Updated the `step` computed property to always return 4 for import facility scenarios.
- Modified the `totalSteps` computed property to correctly handle multiple facilities.
- Added a `hasError` state to track permission errors.
- Implemented a `checkPermissions` method to verify user permissions.
- Enhanced error handling in the `handleContinue` method.
@Jeevansm25
Copy link

Jeevansm25 commented Dec 4, 2024 via email

@MisRob
Copy link
Member

MisRob commented Dec 4, 2024

Hi @Jeevansm25, if you follow the guide I posted on running the two instances, one of them is a development server. Open Kolibri from its URL and it should have your latest local updates.

You can consider the other .pex instance just for having the other facility available - generally it shouldn't be problem for it to run on the older version. Let us know if this work, and if not, please post more details on what exactly you're doing.

@Jeevansm25
Copy link

Hi @Jeevansm25, if you follow the guide I posted on running the two instances, one of them is a development server. Open Kolibri from its URL and it should have your latest local updates.

You can consider the other .pex instance just for having the other facility available - generally it shouldn't be problem for it to run on the older version. Let us know if this work, and if not, please post more details on what exactly you're doing.

After making the changes in the code, do we need to build the project first and then run the Kolibri instances, or can we directly run the instances to see the updates reflected?

Looking forward to your clarification.

@MisRob
Copy link
Member

MisRob commented Dec 6, 2024

@Jeevansm25 if you follow the guide step by step and get to "Run your development server", then upon running yarn run devserver or yarn run devserver-hot, you only need to update code and you should see the updates reflected without the need to rebuild the .pex or rerun the development server.

@MisRob
Copy link
Member

MisRob commented Dec 6, 2024

@Jeevansm25 if you haven't yet done so, set up your development server https://kolibri-dev.readthedocs.io/en/develop/getting_started.html#getting-started

@Jeevansm25
Copy link

@Jeevansm25 if you haven't yet done so, set up your development server https://kolibri-dev.readthedocs.io/en/develop/getting_started.html#getting-started

Of course, I followed all these steps, but I didn't see the changes I made on the development server reflected in the instance running through the .pex file. Could you please clarify this step for me?

@MisRob
Copy link
Member

MisRob commented Dec 9, 2024

I didn't see the changes I made on the development server reflected in the instance running through the .pex file.

@Jeevansm25 Yes, you won't see the changes on the .pex instance. You'd need to build .pex locally for that. I've never done that but could ask others if needed. However, for the majority of tasks it's not needed, perhaps only in some nuanced cases that have to do something with synchronization. I am not sure if this issue is such a case. Would you tell a bit more about why you think you need this?

@Jeevansm25
Copy link

I didn't see the changes I made on the development server reflected in the instance running through the .pex file.

@Jeevansm25 Yes, you won't see the changes on the .pex instance. You'd need to build .pex locally for that. I've never done that but could ask others if needed. However, for the majority of tasks it's not needed, perhaps only in some nuanced cases that have to do something with synchronization. I am not sure if this issue is such a case. Would you tell a bit more about why you think you need this?

Thank you for clarifying about the .pex instance! I was testing a synchronization issue where "Responsibilities of administrator" appears twice when importing from a device with multiple facilities. I wanted to confirm if this issue was fixed by my development changes. Since the .pex instance doesn’t reflect the changes, I now understand that I might need to build a new .pex file. Could you guide me if building it locally is a standard process here, or should I explore other approaches?

@MisRob
Copy link
Member

MisRob commented Dec 9, 2024

I wanted to confirm if this issue was fixed by my development changes.

@Jeevansm25 I'm trying to understand why you can't confirm it on the instance with the development server - what's the nature of your fix? Hopefully then I can then recommend some alternatives if needed. Regarding rebuilding pex, I need to check on this, will get back to you later.

@MisRob
Copy link
Member

MisRob commented Dec 9, 2024

@Jeevansm25 regarding .pex builds, if it's really needed then my colleague recommends

pip install -r requirements/build.txt
make dist
make pex

@MisRob
Copy link
Member

MisRob commented Dec 17, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) bug Behavior is wrong or broken DEV: frontend help wanted Open source contributors welcome P2 - normal Priority: Nice to have
Projects
None yet
Development

No branches or pull requests

5 participants