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.

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

APERTA-11767 Downgrade Ember-data from 2.12.2 to 2.11.3 #3693

Merged
merged 1 commit into from
Oct 31, 2017

Conversation

surfacedamage
Copy link

@surfacedamage surfacedamage commented Oct 31, 2017

JIRA issue: https://jira.plos.org/jira/browse/APERTA-11767

What this PR does:

Ember Data 2.12 introduced a bug which prevents destroys and saves
happening within the same run loop. We had originally updated Ember to
2.13 and then found this issue when testing repeaters which routinely
destroy and save within the same run loop, thus exposing the problem.

Since this bug is subtle and may be present in unknown places elsewhere
in the codebase, it was determined that downgrading to 2.11.* makes the
most sense as a temporary fix until the project can eventually be updated
to the latest version of Ember Data.

We originally attempted upgrading to several NEWER version of Ember Data
(instead of downgrading), but there were many other test failures that
showed up and not enough time to fix the root causes, so this is a
sensible temporary fix.

bug introduced = emberjs/data#4668
bug reported = emberjs/data#4993
bug fixed = emberjs/data#4994

Special instructions for Review or PO:

This is a regression level test.


Code Review Tasks:

Author tasks (delete tasks that don't apply to your PR, this list should be finished before code review):

  • I have ensured that the Heroku Review App has successfully deployed and is ready for PO UAT.

Reviewer tasks (these should be checked or somehow noted before passing on to PO):

  • I read through the JIRA ticket's AC before doing the rest of the review
  • I ran the code (in the review environment or locally). I agree the running code fulfills the Acceptance Criteria as stated on the JIRA ticket
  • I read the code; it looks good

Ember Data 2.12 introduced a bug which prevents destroys and saves 
happening within the same run loop.  We had originally updated Ember to 
2.13 and then found this issue when testing repeaters which routinely 
destroy and save within the same run loop, thus exposing the problem.

Since this bug is subtle and may be present in unknown places elsewhere
in the codebase, it was determined that downgrading to 2.11.* makes the
most sense as a temporary fix until the project can eventually be updated
to the latest version of Ember Data.

We originally attempted upgrading to several NEWER version of Ember Data
(instead of downgrading), but there were many other test failures that
showed up and not enough time to fix the root causes, so this is a 
sensible temporary fix.

bug introduced = emberjs/data#4668
bug reported = emberjs/data#4993
bug fixed = emberjs/data#4994
@plos-ci-agent plos-ci-agent temporarily deployed to plos-ciagent-pr-3693 October 31, 2017 14:51 Inactive
@surfacedamage surfacedamage changed the title Downgrade Ember from 2.12.2 to 2.11.3 APERTA-11767 Downgrade Ember from 2.12.2 to 2.11.3 Oct 31, 2017
@ceautery ceautery changed the title APERTA-11767 Downgrade Ember from 2.12.2 to 2.11.3 APERTA-11767 Downgrade Ember-data from 2.12.2 to 2.11.3 Oct 31, 2017
@JackLaBarba JackLaBarba self-requested a review October 31, 2017 16:22
@JackLaBarba JackLaBarba self-assigned this Oct 31, 2017
@JackLaBarba
Copy link
Contributor

Looks like the correct ED version in the review app:

plos-ciagent-pr-3693 2017-10-31 12-21-30

I gather that there's no know way to reproduce this bug on master, but I'll do some clicking around in the review app to make sure nothing is obviously broken.

So far this all looks good! Thanks for whipping this PR together so quickly @surfacedamage.

Copy link
Contributor

@JackLaBarba JackLaBarba left a comment

Choose a reason for hiding this comment

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

Ok, I did some clicking around and things look good. It appears that doc/docx uploads are broken due to an ihat configuration issue across all review apps, but it's not something that has anything to do with this change.

I think this is good to move on 👍

@egh egh merged commit 740724c into master Oct 31, 2017
@egh egh deleted the features/APERTA-11767-run-loop-deletions branch October 31, 2017 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants