Skip to content

Conversation

@cahrens
Copy link

@cahrens cahrens commented Dec 20, 2013

This fixes 2 issues sited in STUD-868:

  1. poll_module did not handle unicode characters in the answer text. I modified the toy course so that existing "roundtrip" export tests cover this fix.

  2. Fix the logging exception message that is currently breaking UT.2.01x.

@adampalay @singingwolfboy Please review.

@zubair-arbi This bug may also interest you.

@brianhw
Copy link
Contributor

brianhw commented Dec 20, 2013

LGTM. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this is a test. Never mind.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I added it to the existing toy test course so that it covered the bug fix (a unit test fails without the fix).

@cahrens
Copy link
Author

cahrens commented Dec 27, 2013

@dmitchell Can you review so I can go ahead and merge this? Brian's thumbs-up was on an earlier version.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised we're grinding the xml out literally rather than using a library like etree. I don't know what different it would make to use a library tho (I'm wondering if it handles the {text} payload better if it has tags and such). (the code would be

  # if self.question is parsable xml
  xml_object = etree.Element(self._tag_name)
  xml_object.append(etree.fromstring(self.question))
  # if self.question is just text
  xml_object = etree.Element(self._tag_name, text=self.question)
  xml_object.set('display_name', self.display_name)
  ...

but that assumes self.question is itself parsable xml. Otherwise

Copy link
Author

Choose a reason for hiding this comment

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

This is interesting, and I did play around with it. However, it does not appear to be straightforward.

I don't think self.question is necessarily parsable XML (and using the first suggested option leads to a unit test error related to unexpected end of document).

Using text=self.question stores the question as a text attribute, which is different from how it is currently stored. This leads to roundtrip export unit test failures (different failures from the first suggested option).

It does appear that using etree properly handles unicode. Nonetheless, with my very limited understanding of this code, I think that changing how the XML is generated is beyond the scope of this bug fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's ok. I can hold my nose and fully agree you're not responsible :-)

As to how to do it, if self.question may sometimes be parsable xml, then surround the fromstring call w/ try/except and do xml_object.set('text', self.question) in the except (I think). BUT, I'm cool w/ leaving it as is (do no harm).

@dmitchell
Copy link
Contributor

If you think the reset, name, and any other original attrs are handled correctly, 👍

cahrens pushed a commit that referenced this pull request Dec 27, 2013
Bug fixes related to unicode support in export workflow.
@cahrens cahrens merged commit 719a898 into master Dec 27, 2013
@cahrens cahrens deleted the christina/export-bug branch December 27, 2013 16:34
shimulch pushed a commit to open-craft/openedx-platform that referenced this pull request Jan 26, 2021
shimulch pushed a commit to open-craft/openedx-platform that referenced this pull request Jan 26, 2021
* MCKIN-22396 Fix samesite cookie issue for Scorm Shell login flow (openedx#1879)

* Adds django-cookies-sameseite middleware

and settings to set SameSite=None for all secure cookies.

* Fix `SameSite` cookie issue for LTI Provider. EW-449

(cherry picked from commit 28479a2)

* Fix tests, conflicts resolved

Co-authored-by: Jillian Vogel <jill@opencraft.com>
Co-authored-by: Zachary Trabookis <ztraboo@clemson.edu>

* MCKIN-19584: add support for inline discussions translations (openedx#1863)

* MCKIN-19584: add support for inline discussions translations (openedx#1830)

* add support for inline discussions translations

* mark js string for translation on discussions

* ironwood fix

* MCKIN-19840 EOC (End of course Journal) - None of the strings are translated in any language (openedx#1857)

* MCKIN-19840 Add version bump for EOC 0.7.1 (openedx#1867)

* MCKIN-23062 purge cohorts of deleted groups

* MCKIN-23098 404 error on opening the module having EOC with no translations (openedx#1870)

* MCKIN-23098 404 error on opening the module having EOC with no translations integrated

* fix typo and chinese translation on discussions page (openedx#1875)

* Fix mismatching versions for api integration

* security fixes patch

* MCKIN-23604 Match fork code with upstream (openedx#1880)

* [MCKIN-21879] Remove foreign keys from deprecated `progress-edx-platform-extensions` (openedx#1874)

* MCKIN-22507 Updated GW submission API to accept TA email id (openedx#1871)

* MCKIN-22507 Updated GW submission API to accept TA email id

* Fixed typo

* MCKIN-22509 VB

* [MCKIN-22507] Review version bump for xblock-group-project-v2 (openedx#1887)

* MCKIN-22509 Added API to check/create anonymous user id (openedx#1882)

* MCKIN-23249 Threw error if any of group has submissions (openedx#1884)

* Added ignore condition for generated files

* Release freeze (openedx#1895)

* [MCKIN-21879] Remove foreign keys from deprecated `progress-edx-platform-extensions` (openedx#1874)

* MCKIN-22507 Updated GW submission API to accept TA email id (openedx#1871)

* MCKIN-22507 Updated GW submission API to accept TA email id

* Fixed typo

* MCKIN-22509 VB

* [MCKIN-22507] Review version bump for xblock-group-project-v2 (openedx#1887)

* MCKIN-22509 Added API to check/create anonymous user id (openedx#1882)

* MCKIN-23249 Threw error if any of group has submissions (openedx#1884)

* Added ignore condition for generated files

Co-authored-by: Piotr Surowiec <piotr@opencraft.com>
Co-authored-by: Nasir Hussain <nasirhjafri@gmail.com>
Co-authored-by: Sohaib Aslam <sohaib.aslam@arbisoft.com>
Co-authored-by: Shafqat Farhan <shafqat.farhan@arbisoft.com>

* Revert "[MCKIN-21879] Remove foreign keys from deprecated `progress-edx-platform-extensions` (openedx#1874)"

This reverts commit 5dad4f0.

* Revert "[MCKIN-21879] Remove foreign keys from deprecated `progress-edx-platform-extensions` (openedx#1874)"

This reverts commit 5dad4f0.

* MCKIN-23061 Fix enlarged buttons on studio (openedx#1892)

* Revert "Updated subsections grade percentage to start rounding"

This reverts commit fde830b.

* MCKIN-22514 problem builder verion bump

* [MCKIN-21879] Remove foreign keys from deprecated `progress-edx-platform-extensions`

* [BB-2765] Add `reverse_sql` to `database_fixups/0002` migration

* MCKIN-24492 version bump api-integration (openedx#1898)

* MCKIN-21527 Problem Builder (FTE, Assessment, MCQ, MRQ) - On opening these modules, none of the text is translated on notifications dropdown. (openedx#1889)

* MCKIN-21918 Inline discussion japanese buttons (openedx#1891)

Co-authored-by: murad-hubib <murad.habib@arbisoft.com>

* bump xblock problem-builder version to 3.5.1 (openedx#1899)

* MCKIN-24501 api-integration version bump (openedx#1903)

* Security patch from aug 26 (openedx#1894)

Co-authored-by: Ned Batchelder <ned@edx.org>

* sustaining xss security fixes 3 (openedx#1893)

Co-authored-by: Ali-D-Akbar <aliakbarit019@gmail.com>

* Sustaining security fixes 4 & 5 (openedx#1901)

Co-authored-by: Ali-D-Akbar <aliakbarit019@gmail.com>

* MCKIN-24663 sorting alphanumeric list (openedx#1908)

* MCKIN-24633 releasing problem builder v3.5.2 (openedx#1917)

* MCKIN-24559 - Organization extensions VB (openedx#1905)

Releasing `v2.0.6`

* MCKIN-24565 - Organizations extensions VB (openedx#1907)

Releasing `v2.0.7`

* MCKIN-24511 Projects extension VB (openedx#1915)

* Resolved conflits

* Resloved conflicts

* MCKIN-21919 Inline Discussion - The following strings are not translated in Korean Language (openedx#1906)

* Resolved conflict

* Resolved conflicts

* MCKIN-24633 releasing problem builder v3.5.2 (openedx#1917)

* MCKIN-25170 translation for review grade

* MCKIN-24296 missing translations added (openedx#1926)

* missing translations added (openedx#1927)

* MCKIN-25091 fixed Discussion post Anonymously (openedx#1925)

* VB for VR and eoc (openedx#1933)

* MCKIN-25752 discussion and question posted translations (openedx#1934)

* MCKIN-26123 Plateform Issues (openedx#1939)

* MCKIN-26123 Plateform Issues (openedx#1940)

* MCKIN-26123 Drag and Drop Version update (openedx#1942)

* MCKIN-23104 version upgrade of EOC journal (openedx#1941)

* MCKIN-26505 Version update of Problem Builder (openedx#1945)

* MCKIN-26123 Drag&Drop Version Update (openedx#1947)

* MCKIN-26589 Integrate translations of FIND DISCUSSIONS for main discussions page for all languages (openedx#1949)

* [v1.54.0] complied mo files

* MCKIN-24786 - api-integration version bump

* MCKIN-26967 - EOC VB

* MCKIN-23037 - Ooyala xblock version bump

* MCKIN-27119 - Missing strings added

* MCKIN-26468 - projects extensions VB (openedx#1951)

Releasing `v3.0.4`

* MCKIN-24382 Scorm VB (openedx#1966)

* Scorm VB

* MCKIN-26967 EOC VB

* MCKIN-24385: bump xblock diagnostic feedback version to v0.4.0 (openedx#1971)

* MCKIN-24468 McKA - Update Scorm xblock to Python 3.5 fix bug (openedx#1979)

* MCKIN-24385 bump xblock-diagnosticfeedback version to v0.4.1 (openedx#1987)

* MCKIN-26471 API integration VB (openedx#1995)

* bump xblock scorm version to v3.2.1 (openedx#1996)

* MCKIN-26850 EOC-journal version bump (openedx#2001)

* MCKIN-24383 & 24384 VB for GP v2 and Adventure (openedx#1961)

* MCKIN-24383 GW v2 version bump (openedx#2000)

* vb gp (openedx#2002)

* vb gp (openedx#2004)

* MCKIN-28139 Pinned pyopenssl version to 18.0.0

* vb gp (openedx#2004)

* MCKIN-28122 Traslations for all languages (openedx#2015)

* MCKIN-28123 vb gp (openedx#2018)

* Versions update.

Co-authored-by: Moeez Zahid <moeezzahid1996@gmail.com>
Co-authored-by: Jillian Vogel <jill@opencraft.com>
Co-authored-by: Zachary Trabookis <ztraboo@clemson.edu>
Co-authored-by: Saqib <msaqib52@gmail.com>
Co-authored-by: Wasif ur Rehman <wasifarbisoft@users.noreply.github.com>
Co-authored-by: Mudassir Hafeez <mudassir.hafeez@arbisoft.com>
Co-authored-by: Shafqat Farhan <shafqat.farhan@arbisoft.com>
Co-authored-by: ihtram <iulhaq@edx.org>
Co-authored-by: Piotr Surowiec <piotr@opencraft.com>
Co-authored-by: Nasir Hussain <nasirhjafri@gmail.com>
Co-authored-by: Sohaib Aslam <sohaib.aslam@arbisoft.com>
Co-authored-by: Agrendalath <piotr@surowiec.it>
Co-authored-by: Mudassir Hafeez <41048311+mudassir-hafeez@users.noreply.github.com>
Co-authored-by: murad-hubib <murad.habib@arbisoft.com>
Co-authored-by: Ned Batchelder <ned@edx.org>
Co-authored-by: Ali-D-Akbar <aliakbarit019@gmail.com>
Co-authored-by: Naeem Ilyas <naeem-ilyas@live.com>
Co-authored-by: Muhammad Usman <43761905+musmanmalik@users.noreply.github.com>
Co-authored-by: Mudassir Hafeez <mudasir5761@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants