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

[ENG-601] move tasks to ember object extend #695

Conversation

jamescdavis
Copy link
Member

Purpose

Assigning computed properties (which ember-concurrency tasks are) directly to class fields does not work in Ember 3.6+. This moves any of these assignments to extend() or, in a few cases where .extend() could not be used because we are extending a native class, directly to the prototype using Object.defineProperties.

Summary of Changes

  • move tasks in paginated-list/all, paginated-list/has-many, and registries overview route to Object.defineProperties() because they extend from ES6 classes.
  • move all other tasks to .extend()

Side Effects

None expected.

Feature Flags

n/a

QA Notes

No specific QA needed.

Ticket

https://openscience.atlassian.net/browse/ENG-601

Reviewer Checklist

  • meets requirements
  • easy to understand
  • DRY
  • testable and includes test(s)
  • changes described in CHANGELOG.md

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3042

  • 145 of 203 (71.43%) changed or added relevant lines in 24 files are covered.
  • 144 unchanged lines in 18 files lost coverage.
  • Overall coverage decreased (-0.04%) to 72.457%

Changes Missing Coverage Covered Lines Changed/Added Lines %
app/services/ready.ts 5 6 83.33%
lib/osf-components/addon/components/paginated-list/has-many/component.ts 11 12 91.67%
lib/registries/addon/discover/controller.ts 13 14 92.86%
app/dashboard/controller.ts 9 11 81.82%
app/institutions/controller.ts 0 2 0.0%
app/meetings/detail/route.ts 0 2 0.0%
lib/analytics-page/addon/components/analytics-charts/x-chart-wrapper/component.ts 9 11 81.82%
lib/collections/addon/guid/route.ts 1 3 33.33%
lib/collections/addon/submit/route.ts 1 3 33.33%
lib/osf-components/addon/components/sign-up-form/component.ts 3 5 60.0%
Files with Coverage Reduction New Missed Lines %
app/institutions/controller.ts 1 81.03%
app/guid-file/route.ts 1 84.62%
app/services/analytics.ts 1 86.96%
app/services/ready.ts 1 90.7%
lib/registries/addon/discover/controller.ts 1 88.07%
lib/analytics-page/addon/components/analytics-charts/x-chart-wrapper/component.ts 2 82.93%
app/guid-user/quickfiles/route.ts 2 68.52%
app/guid-node/registrations/controller.ts 3 80.9%
lib/osf-components/addon/components/validated-model-form/component.ts 3 81.63%
lib/osf-components/addon/components/tos-consent-banner/component.ts 3 78.85%
Totals Coverage Status
Change from base Build 3033: -0.04%
Covered Lines: 6261
Relevant Lines: 7801

💛 - Coveralls

@jamescdavis
Copy link
Member Author

We should use machty/ember-concurrency-decorators#56 instead

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.

2 participants