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

[BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty #19792

Closed
HardikGoyal2003 opened this issue Feb 20, 2024 · 20 comments · Fixed by #20241
Closed

[BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty #19792

HardikGoyal2003 opened this issue Feb 20, 2024 · 20 comments · Fixed by #20241
Assignees
@alice21mota
Labels
bug Label to indicate an issue is a regression Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@HardikGoyal2003
Copy link
Contributor

HardikGoyal2003 commented Feb 20, 2024 •
edited

Describe the bug

When using tab interaction, if the new tab is added it can be saved empty but when we try to save the content it produces the unexpected error.

ERROR:root:Exception raised at http://localhost:8181/createhandler/autosave_draft/Ap35HNgkJ2ga: No collapsible content is present inside the tag.
Traceback (most recent call last):
  File "/home/hardik/opensource/oppia/core/controllers/editor.py", line 1478, in put
    exp_services.create_or_update_draft(
  File "/home/hardik/opensource/oppia/core/domain/exp_services.py", line 3096, in create_or_update_draft
    updated_exploration = apply_change_list(exp_id, change_list)
  File "/home/hardik/opensource/oppia/core/domain/exp_services.py", line 877, in apply_change_list
    raise e
  File "/home/hardik/opensource/oppia/core/domain/exp_services.py", line 537, in apply_change_list
    content.validate()
  File "/home/hardik/opensource/oppia/core/domain/state_domain.py", line 3491, in validate
    html_cleaner.validate_tabs_and_collapsible_rte_tags(self.html)
  File "/home/hardik/opensource/oppia/core/domain/html_cleaner.py", line 595, in validate_tabs_and_collapsible_rte_tags
    raise utils.ValidationError(
core.utils.ValidationError: No collapsible content is present inside the tag.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/tmpUThEfc/lib/python3.8/site-packages/webapp2.py", line 604, in dispatch
    return method(*args, **kwargs)
  File "/home/hardik/opensource/oppia/core/controllers/acl_decorators.py", line 2134, in test_can_save
    return handler(self, exploration_id, **kwargs)
  File "/home/hardik/opensource/oppia/core/controllers/editor.py", line 1487, in put
    raise self.InvalidInputException(e)
core.controllers.base.UserFacingExceptions.InvalidInputException: No collapsible content is present inside the tag.
ERROR:root:Exception raised: No collapsible content is present inside the tag.
Traceback (most recent call last):
  File "/home/hardik/opensource/oppia/core/controllers/editor.py", line 1478, in put
    exp_services.create_or_update_draft(
  File "/home/hardik/opensource/oppia/core/domain/exp_services.py", line 3096, in create_or_update_draft
    updated_exploration = apply_change_list(exp_id, change_list)
  File "/home/hardik/opensource/oppia/core/domain/exp_services.py", line 877, in apply_change_list
    raise e
  File "/home/hardik/opensource/oppia/core/domain/exp_services.py", line 537, in apply_change_list
    content.validate()
  File "/home/hardik/opensource/oppia/core/domain/state_domain.py", line 3491, in validate
    html_cleaner.validate_tabs_and_collapsible_rte_tags(self.html)
  File "/home/hardik/opensource/oppia/core/domain/html_cleaner.py", line 595, in validate_tabs_and_collapsible_rte_tags
    raise utils.ValidationError(
core.utils.ValidationError: No collapsible content is present inside the tag.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/tmpUThEfc/lib/python3.8/site-packages/webapp2.py", line 604, in dispatch
    return method(*args, **kwargs)
  File "/home/hardik/opensource/oppia/core/controllers/acl_decorators.py", line 2134, in test_can_save
    return handler(self, exploration_id, **kwargs)
  File "/home/hardik/opensource/oppia/core/controllers/editor.py", line 1487, in put
    raise self.InvalidInputException(e)
core.controllers.base.UserFacingExceptions.InvalidInputException: No collapsible content is present inside the tag.

URL of the page where the issue is observed.

N/A

Steps To Reproduce

  1. Visit the Exploration Editor in creator dashboard
  2. Add tab interaction with adding a third tab empty

Expected Behavior

It should handle this error by putting a check for empty content in tab interaction
N/A

Steps To Reproduce

  1. Visit the Exploration Editor in creator dashboard
  2. Add tab interaction as showed in the video

Expected Behavior

It should handle this error by putting a check for empty content in tab interaction

Screenshots/Videos

scrnli_21_02_2024_02-11-17.mp4

What device are you using?

Desktop

Operating System

Linux

What browsers are you seeing the problem on?

Chrome

Browser version

No response

Additional context

No response

Tips for developers

Before addressing the bug, please identify which PR caused the issue (you can follow the steps here). If you identify the PR, comment on the issue with a link to it. If not, mention the commit hash of the oldest commit you saw the bug on (and the month and year it was made in).

Also, if this is your first issue, please make sure to follow https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue and https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#setting-things-up before claiming it. Thanks!
@HardikGoyal2003 HardikGoyal2003 added triage needed bug Label to indicate an issue is a regression labels Feb 20, 2024
@HardikGoyal2003 HardikGoyal2003 changed the title [BUG]: Unexpected error on using empty tab interaction [BUG]: Unexpected error on using empty tab interaction or collapsible interaction Feb 20, 2024
@Happyashbunny
Copy link

hey there @HardikGoyal2003 I'd be interested in solving this issue, can you please direct me to the folder where I can find the code for this

@HardikGoyal2003
Copy link
Contributor Author

Hey @Happyashbunny , Please first can you confirm this is reproducible?

@Happyashbunny
Copy link

@HardikGoyal2003 I tried doing that through the link you posted but it doesn't seem to work.

@HardikGoyal2003
Copy link
Contributor Author

@Happyashbunny I have updated the steps to reproduce, Kindly check. Thanks!

@Happyashbunny
Copy link

@HardikGoyal2003 Thanks for updating the steps. The issue is reproducible. Moreover, the issue only persists if in the newly added tab if either the title or content is left empty.

@HardikGoyal2003
Copy link
Contributor Author

@Happyashbunny Thanks for the confirmation. If you'd like to claim this issue, then, per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue.

Please also follow the other instructions on that wiki page if you have not yet done so. Thanks!

@seanlip seanlip added Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Feb 25, 2024
@HardikGoyal2003 HardikGoyal2003 changed the title [BUG]: Unexpected error on using empty tab interaction or collapsible interaction [BUG]: Unexpected error on using empty tab RTE component or collapsible RTE component Feb 25, 2024
@HardikGoyal2003 HardikGoyal2003 changed the title [BUG]: Unexpected error on using empty tab RTE component or collapsible RTE component [BUG]: Unexpected error on using empty tab interaction or collapsible interaction Feb 25, 2024
@HardikGoyal2003 HardikGoyal2003 changed the title [BUG]: Unexpected error on using empty tab interaction or collapsible interaction [BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty Feb 25, 2024
@kevintab95
Copy link
Member

This error occurs on the 3.3.5 branch too.

@joanapeixinho
Copy link
Contributor

Hello @HardikGoyal2003 I'm interested in solving this issue and I can reproduce the bug.
But first, I would like to better understand the expected behaviour: "It should handle this error by putting a check for empty content in tab interaction". Where should this check be?

@HardikGoyal2003
Copy link
Contributor Author

HardikGoyal2003 commented Mar 6, 2024 •
edited

Hey @joanapeixinho Thanks for showing interest in solving this issue.
The root cause of the problem is that we can save the tab RTE component or collapsible RTE component as empty. As soon as we process that code, it fails because it cannot handle the case where the component is empty. So, what I mean by 'check' is that when we take input from the user, we can apply a condition in the code that the input field must contain some printable characters. If it is empty, then the 'Done' button must be disabled for that time so that the user cannot submit the component. It must also show a text if the user is trying to submit empty.

@joanapeixinho
Copy link
Contributor

Hi! @HardikGoyal2003

I would like to propose an approach to this issue. I would alter the files core/templates/services/rte-helper-modal.controller.ts and core/templates/services/rte-helper-modal.component.html.

In the rte-helper-modal.controller.ts I would add a function called disableSaveButtonForTabContents that would basically check if the title or contents are empty and if so, would disable the save button.

Then in the rte-helper-modal.component.html i would add that condition to the Done button much like what we already have for when Rte is a Math Expression Editor.

Does this seem like a good approach?

Thank you! 😊

@HardikGoyal2003
Copy link
Contributor Author

@seanlip /cc, to me it seems fine as the link RTE component and math RTE component disable buttons from these files only. Thanks!

@seanlip
Copy link
Member

seanlip commented Mar 9, 2024

@joanapeixinho Ah, I have a request: could you please work on #18211 first, if that's OK? It sets up a more general structure for RTE component validation, and then we can work this into that. I think that would be a better order of doing things. A lot of the work has been done already for that issue so it's just a question of finishing it up, and then I think that will lead to this being a simple addition to that.

What do you think?

@joanapeixinho
Copy link
Contributor

@seanlip Yes, I would like to work on that issue! I will suggest a solution as soon as I can. Thank you for the help!

@alice21mota
Copy link
Contributor

Hello @seanlip!
Based on the PR related to issue #18211, I think a possible solution for this issue would be to add a verification in rte-helper-modal.controller.ts for when the title or content of a tab are not filled. By using that PR as starting point, it is possible to ensure consistency in the RTE component validation.
Here is video proof of my solution working, as well as the code I added.

Screen.Recording.2024-03-25.at.10.13.15.mov
Screenshot 2024-03-25 at 10 14 55

@seanlip
Copy link
Member

seanlip commented Mar 25, 2024

@alice21mota Yup, something along those lines sounds good to me. Feel free to take this up once the other PR gets merged!

@alice21mota
Copy link
Contributor

Thank you @seanlip !
Can I please be assigned this issue then? Or only after the PR gets merged?

@seanlip seanlip changed the title [BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty [Blocked on #18211] [BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty Mar 25, 2024
@seanlip
Copy link
Member

seanlip commented Mar 25, 2024

Only after the other PR gets merged. I suggest taking up something else for now, thanks!

alice21mota added a commit to alice21mota/oppia that referenced this issue Apr 5, 2024
@alice21mota
Fix oppia#19792: Enhance Validation for tab component in Exploration …
b47fc36 
…Editor

closes oppia#19792
@alice21mota
Copy link
Contributor

alice21mota commented Apr 27, 2024 •
edited

Hi @seanlip ! I see PR #20071 has been accepted and on queue to merge. Any clues on when I will be able to be assigned to this issue? Thank you :)

@seanlip
Copy link
Member

seanlip commented Apr 29, 2024

Hi @alice21mota, yup assigning you. Go for it, thanks!

@seanlip seanlip changed the title [Blocked on #18211] [BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty [BUG]: Unexpected error on using tab RTE component or collapsible RTE component empty Apr 29, 2024
@alice21mota alice21mota mentioned this issue Apr 30, 2024
Merged
5 tasks
@alice21mota
Copy link
Contributor

Hi @seanlip !
I already made a PR. Thank you for your attention :)

github-merge-queue bot pushed a commit that referenced this issue May 14, 2024
@alice21mota @seanlip
Fix #19792: raise error when tab RTE components are empty and prevent…
59f92d5 
… saving (#20241)

* Fix: Handle empty content error in tab interaction

Prevents user from saving when a component of a tab is empty, displaying an error message

* fix: linting errors and unnecessary code removed

* fix: changes request by reviwer
dquote> change comment related to Value[0] in line 257 to be more specific
dquote> replace tab with tabIndex
dquote> make error messages more explicit for users
dquote> fix tests to accomodate changes

* fix: changes request by reviwer
dquote> change comment related to Value[0] in line 257 to be more specific
dquote> replace tab with tabIndex
dquote> make error messages more explicit for users
dquote> fix tests to accomodate changes

* fix: changes request by reviwer
 change comment related to Value[0] in line 257 to be more specific
 replace tab with tabIndex
 make error messages more explicit for users
 fix tests to accomodate changes

* fix: displayed errors messages
alter displayed messages when a tab component is not filled
change corresponding tests accordingly
fix comment typos

---------

Co-authored-by: Sean Lip <sean@seanlip.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alice21mota alice21mota

Labels
bug Label to indicate an issue is a regression Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
LaCE Quality Team
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #19792: raise error when tab RTE components are empty and prevent saving alice21mota/oppia
6 participants
@seanlip @kevintab95 @joanapeixinho @alice21mota @Happyashbunny @HardikGoyal2003