Skip to content

Conversation

@bradenmacdonald
Copy link
Contributor

@bradenmacdonald bradenmacdonald commented Aug 1, 2023

Description

This PR implements openedx/modular-learning#19 .

Normally, the Studio outline view looks like this:

Screenshot 2023-08-01 at 12 14 45 PM

When the new Waffle flag contentstore.enable_copy_paste_units is enabled, most of the actions get pushed into the new "Actions menu":

Screenshot 2023-08-01 at 12 16 47 PM

This is analogous to the change for components that happened in #31853

As you can see, when the new menu is enabled, there is also a new Copy to Clipboard action that can be used to copy/paste units, either within a course or among different courses.

Supporting information

See ticket linked above.

Testing instructions

  1. Check out this branch, rebuild static assets, etc.
  2. Go to a course outline page and make sure the action buttons look "normal"
  3. Go to http://studio.local.overhang.io:8001/admin/waffle/flag/ or devstack equivalent, and add a new flag called contentstore.enable_copy_paste_units which is enabled for Everyone.
  4. Go to a course outline page and observe the new actions menu. Try copying and pasting a unit, within a course and between different courses.
  5. Make sure the "Paste Unit" button disappears when a regular (non-unit) component is copied to the clipboard and vice versa. Check that this happens instantly across all open tabs.

Deadline

None

Other information

Depends on #32812

Private-ref: MNG-3771

@openedx-webhooks
Copy link

openedx-webhooks commented Aug 3, 2023

Thanks for the pull request, @bradenmacdonald! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 3, 2023
@yusuf-musleh yusuf-musleh force-pushed the braden/copy-units-ui branch 2 times, most recently from 633246e to cbe0bd7 Compare August 8, 2023 14:41
@bradenmacdonald bradenmacdonald marked this pull request as ready for review August 10, 2023 23:48
@openedx-webhooks
Copy link

Thanks for the pull request, @bradenmacdonald! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

@bradenmacdonald, some notes for the follow-up tickets:

  1. Copying the Python library works correctly when we copy a single Problem XBlock. However, if we copy the entire Unit, this zip file is no longer copied to the clipboard.
  2. Copying the LibraryContentBlock raises an exception. If we are not planning to support it eventually, we should at least handle this use case gracefully while copying the Unit. Otherwise, only the XBlocks placed before the LibraryContentBlock are pasted.
  3. I'm randomly getting the following exceptions while loading a pasted Unit. This Unit contains SGA XBlocks with static assets (some present in the course files and some non-existent ones) linked in the "Solution" field. It occurs when I reload the page a few times. However, this is probably unrelated, as I'm getting some other Memcached errors since I pulled the most recent changes into my devstack today.
2023-08-14 12:29:47,514 ERROR 2259 [django.request] [user 3] [ip 172.31.0.1] log.py:224 - Internal Server Error: /asset-v1:new+new+new+type@asset+block/absent.jpg
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/deprecation.py", line 116, in __call__
    response = self.process_request(request)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/contentserver/middleware.py", line 84, in process_request
    content = self.load_asset_from_location(loc)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/contentserver/middleware.py", line 288, in load_asset_from_location
    content = get_cached_content(location)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/contentserver/caching.py", line 29, in get_cached_content
    return CONTENT_CACHE.get(str(location).encode("utf-8"), version=STATIC_CONTENT_VERSION)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/cache/backends/memcached.py", line 77, in get
    return self._cache.get(key, default)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/hash.py", line 347, in get
    return self._run_cmd("get", key, default, default=default, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/hash.py", line 322, in _run_cmd
    return self._safely_run_func(client, func, default_val, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/hash.py", line 211, in _safely_run_func
    result = func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 687, in get
    return self._fetch_cmd(b"get", [key], False, key_prefix=self.key_prefix).get(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 1165, in _fetch_cmd
    raise MemcacheUnknownError(line[:32])
pymemcache.exceptions.MemcacheUnknownError: b'z\xd2v\xc8\x05\x04\xa5\x89o\x82\xa9b\xbb\x95?\x94vA\x16\x94t\xb3}\xa1\xfe\xc7\x99+\xa1\x97\xb1X'
2023-08-14 12:29:47,545 ERROR 2259 [django.request] [user 3] [ip 172.31.0.1] log.py:224 - Internal Server Error: /assets/courseware/v1/75c11d787c658add35ad8425c4cd1e3d/asset-v1:new+new+new+type@asset+block/present.webp
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/deprecation.py", line 116, in __call__
    response = self.process_request(request)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/contentserver/middleware.py", line 84, in process_request
    content = self.load_asset_from_location(loc)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/contentserver/middleware.py", line 288, in load_asset_from_location
    content = get_cached_content(location)
  File "/edx/app/edxapp/edx-platform/openedx/core/djangoapps/contentserver/caching.py", line 29, in get_cached_content
    return CONTENT_CACHE.get(str(location).encode("utf-8"), version=STATIC_CONTENT_VERSION)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/cache/backends/memcached.py", line 77, in get
    return self._cache.get(key, default)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/hash.py", line 347, in get
    return self._run_cmd("get", key, default, default=default, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/hash.py", line 322, in _run_cmd
    return self._safely_run_func(client, func, default_val, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/hash.py", line 211, in _safely_run_func
    result = func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 687, in get
    return self._fetch_cmd(b"get", [key], False, key_prefix=self.key_prefix).get(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 1153, in _fetch_cmd
    key, value, buf = self._extract_value(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 1103, in _extract_value
    buf, value = _readvalue(self.sock, buf, int(size))
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 1689, in _readvalue
    buf = _recv(sock, RECV_SIZE)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pymemcache/client/base.py", line 1750, in _recv
    return sock.recv(size)
OSError: [Errno 9] Bad file descriptor

👍

  • I tested this: checked that copying and pasting Units works as expected
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Nit: if we decide to change the styling of these menus, we will need to make changes in two files. Would it make sense to share these rules between the outline and unit views?

https://github.com/open-craft/edx-platform/blob/766fdc5/cms/static/sass/elements/_xblocks.scss#L72-L90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Frankly, I didn't worry about the UI maintainability too much as the whole thing is slated to be replaced by an MFE this Fall/Winter.

Co-authored-by: Piotr Surowiec <piotr@surowiec.it>
@bradenmacdonald bradenmacdonald merged commit 08fda1b into openedx:master Aug 14, 2023
@bradenmacdonald bradenmacdonald deleted the braden/copy-units-ui branch August 14, 2023 18:58
@openedx-webhooks
Copy link

@bradenmacdonald 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FC Relates to an Axim Funded Contribution project open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants