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.

Sign up for GitHub

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

Jump to bottom

SAK-51198 Lessons restore the print all feature #13501

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

SAK-51198 Lessons restore the print all feature #13501

wants to merge 2 commits into from

Conversation

jumarub
Copy link
Contributor

@jumarub jumarub commented Mar 27, 2025

No description provided.

@bbbbgarcia bbbbgarcia marked this pull request as draft March 27, 2025 13:49
bbbbgarcia
bbbbgarcia requested changes Mar 27, 2025
View reviewed changes
Copy link
Contributor

@bbbbgarcia bbbbgarcia left a comment

Choose a reason for hiding this comment

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

Can you please hide it using a property? That way people who don't want to use it will be fine with bringing it back. Thanks!

@jumarub jumarub force-pushed the SAK-51198 branch 2 times, most recently from e47a130 to 34cca9a Compare March 27, 2025 14:31
@jumarub
Copy link
Contributor Author

jumarub commented Mar 27, 2025

Can you please hide it using a property? That way people who don't want to use it will be fine with bringing it back. Thanks!

Done, by default the feature won't be avaliable, you will need to set it to true.

@jumarub jumarub requested a review from bbbbgarcia March 27, 2025 14:32
@ern ern changed the title SAK-51198: Lesson Print All restore SAK-51198 Lesson print all restore Mar 27, 2025
@ern
Copy link
Contributor

ern commented Mar 27, 2025

I remember this being removed because it suffered from significant performance issue that could cause a node to stall. Maybe there should be some protections put in place.

@ern ern changed the title SAK-51198 Lesson print all restore SAK-51198 Lessons restore the print all feature Mar 27, 2025
@jumarub jumarub marked this pull request as ready for review April 3, 2025 06:41
@jumarub jumarub requested a review from ern April 3, 2025 06:42
@stetsche
Copy link
Contributor

stetsche commented Apr 3, 2025

@ern Could you expand on the performance issue? It looks like this would just render the current page in a new window. If that is causing major issues we might be able to reproduce it without this button?

@bbbbgarcia
Copy link
Contributor

@ern Could you expand on the performance issue? It looks like this would just render the current page in a new window. If that is causing major issues we might be able to reproduce it without this button?

I think a related conversation happened in this jira https://sakaiproject.atlassian.net/browse/SAK-50493 (see old comments)

@jumarub jumarub requested a review from stetsche April 7, 2025 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbbbgarcia bbbbgarcia bbbbgarcia approved these changes

@stetsche stetsche stetsche approved these changes

@ern ern Awaiting requested review from ern

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jumarub @ern @stetsche @bbbbgarcia