Skip to content

Fix publishing in old mongo - #1203

Merged
dmitchell merged 2 commits into
masterfrom
dhm/bug_publish
Oct 2, 2013
Merged

Fix publishing in old mongo#1203
dmitchell merged 2 commits into
masterfrom
dhm/bug_publish

Conversation

@dmitchell

@dmitchell dmitchell commented Oct 2, 2013

Copy link
Copy Markdown
Contributor

Fix for STUD-811.

Deleted units weren't being deleted upon publication.

Can @chrisndodge plus one of @cahrens @singingwolfboy please review (or delegate)? CAT-1 bug

@singingwolfboy

singingwolfboy commented Oct 2, 2013

Copy link
Copy Markdown
Contributor

I don't see anything wrong with this. However, I'm still not very familiar with the database layer in our application, so I don't entirely understand it.

@chrisndodge

chrisndodge commented Oct 2, 2013

Copy link
Copy Markdown
Contributor

Can you add a test case which does a move operation and verify that the underlying object is not deleted? I see you correctly identified that as a use-case, but I think it'd be good to test that.

Otherwise looks good.

Thanks.

cahrens
cahrens reviewed Oct 2, 2013
View reviewed changes
Comment thread common/lib/xmodule/xmodule/modulestore/mongo/draft.py Outdated

@cahrens cahrens Oct 2, 2013

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like the rents and the mom. But there's a typo in "original_publlished".

@cahrens

cahrens commented Oct 2, 2013

Copy link
Copy Markdown

A few pep8 things in test_publish. I looked at the code, and it seems reasonable (not that I completely understand it).

I have no desire to re-review. :)

@dmitchell

dmitchell commented Oct 2, 2013

Copy link
Copy Markdown
Contributor Author

@chrisndodge L162 in the test moves one of the children before the publish.

@chrisndodge

chrisndodge commented Oct 2, 2013

Copy link
Copy Markdown
Contributor

Thx. Didn't spot that. I'll defer to you whether that might want to be a dedicated test method or not.

Don Mitchell added 2 commits October 2, 2013 16:06
Don Mitchell
Test reproducing orphaning of children
18c4587 
when draft is deleted and then parent published.
ensure moving doesn't cause deletion
Don Mitchell
Publish removes deleted children
5e1f03b
@dmitchell

dmitchell commented Oct 2, 2013

Copy link
Copy Markdown
Contributor Author

Fixed all pep8 and the one typo in the comment.

@chrisndodge can I get a thumbs up?

@chrisndodge

chrisndodge commented Oct 2, 2013

Copy link
Copy Markdown
Contributor

yep, +1

dmitchell added a commit that referenced this pull request Oct 2, 2013
@dmitchell
Merge pull request #1203 from edx/dhm/bug_publish
0521bec 
Fix publishing in old mongo
@dmitchell
dmitchell merged commit 0521bec into master Oct 2, 2013
@jzoldak
jzoldak deleted the dhm/bug_publish branch May 5, 2014 14:55
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Sep 23, 2016
@sakamaki-y
Merge pull request openedx#1203 from sakamaki-y/develop/dogwood/chang…
97c03e3 
…e_request_of_words_by_marketing_div

Change request of few messages by marketing div openedx#1184
@matiasua matiasua mentioned this pull request Oct 1, 2023
Merged
@sumodgeorge sumodgeorge mentioned this pull request Oct 1, 2023
Open
@baby636 baby636 mentioned this pull request Oct 2, 2023
Open
@matiasua matiasua mentioned this pull request Oct 2, 2023
Closed
iloveagent57 pushed a commit that referenced this pull request Feb 26, 2024
@dependabot @UsamaSadiq
build(deps): Bump peter-evans/create-or-update-comment (#1203)
9826d42 
Bumps [peter-evans/create-or-update-comment](https://github.com/peter-evans/create-or-update-comment) from 46da6c0d98504aed6fc429519a258b951f23f474 to e3645dd16d792dc1461bba740dab47338596a26a.
- [Release notes](https://github.com/peter-evans/create-or-update-comment/releases)
- [Commits](peter-evans/create-or-update-comment@46da6c0...e3645dd)

---
updated-dependencies:
- dependency-name: peter-evans/create-or-update-comment
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Usama Sadiq <usama.sadiq@arbisoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

@dmitchell @singingwolfboy @chrisndodge @cahrens