Skip to content

Conversation

@symbolist
Copy link
Contributor

ORA-286

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be cleaner to modify the DummyRequest class in utils.py but it is used for grading too. So to keep the changes isolated I have defined a new one. I can modify the other one too instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine for now, since it's more conservative.

Copy link
Contributor

Choose a reason for hiding this comment

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

why return at the end of __init__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy pasted. Will remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

why was it in the other dummy request?

@symbolist
Copy link
Contributor Author

@cpennington @feanil Here is the hotfix PR.

@adampalay
Copy link
Contributor

I think this is good to go.
if cale and feanil think so too, let's get this up on staging

Copy link
Contributor

Choose a reason for hiding this comment

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

If hostname is None, then this leaves request as None, and relies on get_module_for_student to create it. Why not just require get_module_for_student to always get a request, and always to request creation up here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really follow: are you saying that we should lose L74-75?

I don't want to change the definition of get_module_for_student because it's used elsewhere in the openended command suite, and I want the amount of moving parts in this hotfix to remain at a minimum.

@cpennington
Copy link
Contributor

Other than my one comment, 👍

Also, we really need to make it so you don't need a request to get an XModule. =P

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need the conditional here, too?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, because there are other commands that use this function

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

that don't pass in request as a param

Copy link
Contributor

Choose a reason for hiding this comment

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

it's not pretty, but again I just wanted to keep moving parts to a minimum for this hotfix

cpennington added a commit that referenced this pull request Jan 15, 2014
Added hostname argument to openended_post command.
@cpennington cpennington merged commit 310b819 into release Jan 15, 2014
@cpennington cpennington deleted the usman/openended_post_hostname_argument branch January 15, 2014 16:55
@cpennington cpennington restored the usman/openended_post_hostname_argument branch January 15, 2014 17:01
@symbolist symbolist deleted the usman/openended_post_hostname_argument branch March 5, 2014 11:50
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Sep 22, 2017
* Add menu to ga_operation for ga_analyzer openedx#2039 (openedx#2088)

* add role for old course viewer openedx#2062 (openedx#2087)

* add role for old course viewer openedx#2062

* Change action for biz course by BetaTester role openedx#2062

* Construction of image server openedx#2025 (openedx#2106)

* cherry-pick 8c8953f

* Fix file upload in IE

* Construction of image server openedx#2025

* add all keywords search in Student management openedx#2029 (openedx#2034)

* Fix bug for before enrollment start in ga old course viewer openedx#2062 (openedx#2125)

* fix. Construction of image server openedx#2025 (openedx#2117)

* Modify message and css of enrollment for Course About openedx#2130

* Add a certificate list to user's profile page. openedx#2042 (openedx#2108)

* Mod UT openedx#2130

* add PDF File Construction of image server openedx#2025 (openedx#2140)

* add library option, and library links to the course. openedx#2001 (openedx#2124)

* Invalid StudioPermissionsService object in API to show/save xblock settings in CMS.
Randomized Content Block editor did not check Studio user's permissions

* add library option, and library links to the course. openedx#2001

* fix. add all keywords search in Student management openedx#2029 (openedx#2034) (openedx#2157)

* second fix. Construction of image server openedx#2025 (openedx#2158)

* add library option, and library links to the course. openedx#2001 (openedx#2160)

* third fix. Construction of image server openedx#2163 (openedx#2164)

* Add filter by category for certificates on profile page openedx#2042 (openedx#2165)

* Fix bug for  add library option, and library links to the course. openedx#2162 openedx#2133 (openedx#2167)

* Develop/dogwood/gacco201708 (openedx#2170)

* Fixed bugs openedx#2039 (openedx#2112)

* Fixed csv format openedx#2039 (openedx#2127)

* Change to split download if there are many display items openedx#916 (openedx#2121)

* Change to split download if there are many display items openedx#916

* Fix UT

* Fix Review

* Fix review2
xitij2000 pushed a commit to open-craft/openedx-platform that referenced this pull request Mar 18, 2021
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.

4 participants