Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

kolibri studio no of remote channel issue resolved #11100

Merged
merged 11 commits into from
Aug 29, 2023

Conversation

GarvitSinghal47
Copy link
Contributor

@GarvitSinghal47 GarvitSinghal47 commented Aug 15, 2023

#10931

Summary

Added the no of remote channel line under the library .

Before :

channelnobefore.webm

After :

channelnoafter.webm

References

Reviewer guidance


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: small labels Aug 15, 2023
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Just need to make sure that we are using the translated string!

@MisRob
Copy link
Member

MisRob commented Aug 18, 2023

Thank you, @GarvitSinghal47. We noticed one more thing here that I will need to look into in more detail than I can manage now so I will return to it on Monday.

Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Hi again, @GarvitSinghal47, this is a good start, thank you! Due to the similarity of some places in Kolibri Library, during the first review we didn't notice a few things, apologies. There will be a few updates needed.

(1) The issue reported in #10931 is related to the missing number of available channels on the "All libraries" page rather than on the root "Library" page. I double-checked designs and we don't want to show the number on the root "Library" page like implemented in this PR. You can follow the guidance in the issue to navigate to the right place, however you'd need to have some other devices on your network to have "View all" button available like here

all-libs

Alternatively, I think that navigating to http://localhost:8000/en/learn/#/explore_libraries?prevName=LIBRARY should do, too.

(2) We only want to display the number of channels for Kolibri Library (Studio) library rather than the total number of channels on remote devices.

Let me know if you needed some help with setting up another device on your network. It's not that complicated and we can support. It might be useful for you to be able to navigate everywhere and also to check that the correct number is calculated.

@GarvitSinghal47
Copy link
Contributor Author

GarvitSinghal47 commented Aug 22, 2023

Hi again, @GarvitSinghal47, this is a good start, thank you! Due to the similarity of some places in Kolibri Library, during the first review we didn't notice a few things, apologies. There will be a few updates needed.

(1) The issue reported in #10931 is related to the missing number of available channels on the "All libraries" page rather than on the root "Library" page. I double-checked designs and we don't want to show the number on the root "Library" page like implemented in this PR. You can follow the guidance in the issue to navigate to the right place, however you'd need to have some other devices on your network to have "View all" button available like here

all-libs

Alternatively, I think that navigating to http://localhost:8000/en/learn/#/explore_libraries?prevName=LIBRARY should do, too.

(2) We only want to display the number of channels for Kolibri Library (Studio) library rather than the total number of channels on remote devices.

Let me know if you needed some help with setting up another device on your network. It's not that complicated and we can support. It might be useful for you to be able to navigate everywhere and also to check that the correct number is calculated.

Oh i realized now after seeing the design docs i have done it in the wrong place, will move it to all library page .

@MisRob i need help in setting it up in my network as while i am trying to access this on my wifi with the url given in device then info , i was able to get the url but the problem i am facing in both my other laptop and the mobile is that the loading page is just appearing and not forwarding to auth page .

and when i inspect the page it is showing err connection refused and uncaught reference error : kolibricore app global is not defined.
and when i run it in the same laptop where localhost is running then it is working fine .

i have tried removing all security related thing of browser and the security software of my laptop as well.

can you tell me what is the issue and how to resolve it .

@MisRob
Copy link
Member

MisRob commented Aug 22, 2023

i need help in setting it up in my network ...

@GarvitSinghal47 just to clarify, you're doing this so you can see more devices in the Library, is that correct?

@MisRob
Copy link
Member

MisRob commented Aug 22, 2023

I will need to move to something else in a moment, so if that's case, I'd recommend a simpler way of running another device alongside your dev server, just one laptop needed :)

You can download pex file from one of the latest Build Assets(if you can access the page), for example this one https://github.com/learningequality/kolibri/suites/15341352681/artifacts/876105184. Then unzip the file and run KOLIBRI_HOME="kolibri-home" python <filename>.pex start (you can choose any location for KOLIBRI_HOME, just don't forget to set it so that you don't override your default .kolibri). This will run another Kolibri instance. In the terminal, you will find its URL. Follow it and do the initial device setup as usual, and also import few resources from some channels. Then, when you run your development server as usual, you should see this other device on your network. When you're done, you can run KOLIBRI_HOME="kolibri-home" python <filename>.pex stop to stop the other Kolibri instance.


When it comes to uncaught reference error : kolibricore app global, what usually helps me is to restart the server or rebuild frontend assets, but I think it might be easier to try to follow the steps above. That's how we commonly do it and I rarely experience issues. Let us know how it goes.

@GarvitSinghal47
Copy link
Contributor Author

i need help in setting it up in my network ...

@GarvitSinghal47 just to clarify, you're doing this so you can see more devices in the Library, is that correct?

Yes , to see more devices .

@GarvitSinghal47
Copy link
Contributor Author

updated video after changes in description

:pinIcon="getPinIcon(true)"
:disablePinDevice="device['instance_id'] === studioId"
Copy link
Member

Choose a reason for hiding this comment

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

This change is perfectly aligned with the designs, but let me check with the team at first before we merge it because disabling pinning the Kolibri Library here is looking pretty intentional so now I am not clear if the deviation from the designs had a reason or if it's just our oversight.

Copy link
Member

Choose a reason for hiding this comment

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

Let's revert this change - the designs are out of date, and this should indeed be disabled instead.

@MisRob MisRob self-requested a review August 25, 2023 15:48
Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Thank you @GarvitSinghal47

@MisRob MisRob dismissed rtibbles’s stale review August 29, 2023 06:53

Feedback addressed

@MisRob MisRob merged commit aac733c into learningequality:release-v0.16.x Aug 29, 2023
@GarvitSinghal47 GarvitSinghal47 deleted the ux-3 branch September 7, 2023 05:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: small SIZE: very small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants