Merge pull request #374 from san650/change-collection-signature

Change collection signature

addon/-private/properties/collection.js

@@ -1,4 +1,5 @@
 import { deprecate } from '@ember/application/deprecations';
+import { warn } from '@ember/debug';
 
 import { collection as mainCollection } from './collection/main';
 import { collection as legacyCollection } from './collection/legacy';
@@ -36,8 +37,7 @@ import { collection as legacyCollection } from './collection/legacy';
  * // </table>
  *
  * const page = PageObject.create({
- *   users: collection({
- *     scope: 'table tr',
+ *   users: collection('table tr', {
  *     firstName: text('td', { at: 0 }),
  *     lastName: text('td', { at: 1 })
  *   })
@@ -72,8 +72,7 @@ import { collection as legacyCollection } from './collection/legacy';
  * const page = PageObject.create({
  *   scope: '.admins',
  *
- *   users: collection({
- *     scope: 'table tr',
+ *   users: collection('table tr', {
  *     firstName: text('td', { at: 0 }),
  *     lastName: text('td', { at: 1 })
  *   })
@@ -100,8 +99,7 @@ import { collection as legacyCollection } from './collection/legacy';
  * const page = PageObject.create({
  *   scope: 'table',
  *
- *   users: PageObject.collection({
- *     scope: 'tr',
+ *   users: PageObject.collection('tr', {
  *     firstName: text('td', { at: 0 }),
  *     lastName: text('td', { at: 1 }),
  *   })
@@ -110,18 +108,29 @@ import { collection as legacyCollection } from './collection/legacy';
  * let john = page.users.filter((item) => item.firstName === 'John' )[0];
  * assert.equal(john.lastName, 'Doe');
  *
- * @param {Object} definition - Collection definition
+ * @param {String} scopeOrDefinition - Selector to define the items of the collection
+ * @param {Object} [definitionOrNothing] - Object with the definition of item properties
  * @return {Descriptor}
  */
-export function collection(definition) {
-  if ('itemScope' in definition) {
-    deprecate('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.', false, {
-      id: 'ember-cli-page-object.old-collection-api',
-      until: '2.0.0'
-    });
+export function collection(scopeOrDefinition, definitionOrNothing) {
 
-    return legacyCollection(definition);
-  } else {
-    return mainCollection(definition);
+  if (typeof scopeOrDefinition === 'string') {
+    return mainCollection(scopeOrDefinition, definitionOrNothing);
   }
+
+  deprecate('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.', false, {
+    id: 'ember-cli-page-object.old-collection-api',
+    until: '2.0.0',
+    url: 'https://gist.github.com/san650/17174e4b7b1fd80b049a47eb456a7cdc#file-old-collection-api-js',
+  });
+
+  warn(
+    'Legacy page object collection definition is invalid. Please, make sure you include a `itemScope` selector.',
+    scopeOrDefinition.itemScope,
+    {
+      id: 'ember-cli-page-object.legacy-collection-missing-item-scope'
+    }
+  );
+
+  return legacyCollection(scopeOrDefinition);
 }

addon/-private/properties/collection/main.js

@@ -10,15 +10,16 @@ import { count } from '../count';
 import Ceibo from 'ceibo';
 
 export class Collection {
-  constructor(definition, parent, key) {
-    this.definition = definition;
+  constructor(scope, definition, parent, key) {
+    this.scope = scope;
+    this.definition = definition || {};
     this.parent = parent;
     this.key = key;
 
     this._itemCounter = create({
-      count: count(definition.scope, {
-        resetScope: definition.resetScope,
-        testContainer: definition.testContainer
+      count: count(scope, {
+        resetScope: this.definition.resetScope,
+        testContainer: this.definition.testContainer
       })
     }, { parent });
 
@@ -33,12 +34,12 @@ export class Collection {
     let { key } = this;
 
     if (typeof this._items[index] === 'undefined') {
-      let { definition, parent } = this;
-      let scope = buildSelector({}, definition.scope, { at: index });
+      let { scope, definition, parent } = this;
+      let itemScope = buildSelector({}, scope, { at: index });
 
       let finalizedDefinition = assign({}, definition);
 
-      finalizedDefinition.scope = scope;
+      finalizedDefinition.scope = itemScope;
 
       let tree = create(finalizedDefinition, { parent });
 
@@ -105,15 +106,15 @@ if (typeof (Symbol) !== 'undefined' && Symbol.iterator) {
   }
 }
 
-export function collection(definition) {
+export function collection(scope, definition) {
   let descriptor = {
     isDescriptor: true,
 
     setup(node, key) {
       // Set the value on the descriptor so that it will be picked up and applied by Ceibo.
       // This does mutate the descriptor, but because `setup` is always called before the
       // value is assigned we are guaranteed to get a new, unique Collection instance each time.
-      descriptor.value = new Collection(definition, node, key);
+      descriptor.value = new Collection(scope, definition, node, key);
     }
   };
 

tests/integration/actions-test.js

@@ -36,18 +36,13 @@ const button = function(scope) {
 };
 
 const page = PageObject.create({
-  numbers: collection({
-    scope: '.numbers',
-    itemScope: 'button',
-
-    item: {
-      click: clickable(),
-      text: text()
-    },
-
-    clickOn: clickOnText()
+  numbers: collection('.numbers button', {
+    click: clickable(),
+    text: text()
   }),
 
+  clickOnNumber: clickOnText('.numbers'),
+
   operators: {
     scope: '.operators',
 
@@ -112,22 +107,22 @@ test('Actions work when defined inside collections', function(assert) {
   this.render(template);
 
   page
-    .numbers(0)
+    .numbers
+    .objectAt(0)
     .click();
 
   assert.equal(page.screen.text, '1');
 });
 
-test('Chaining of actions inside a collection works', function(assert) {
+test('Chaining of custom actions works', function(assert) {
   let template = createCalculatorTemplate();
 
   this.render(template);
 
   page
-    .numbers()
-    .clickOn('1')
-    .clickOn('2')
-    .clickOn('3');
+    .clickOnNumber('1')
+    .clickOnNumber('2')
+    .clickOnNumber('3');
 
   assert.equal(page.screen.text, '123');
 });

tests/integration/deprecations/comma-separated-selector-test.js

@@ -7,7 +7,7 @@ import PageObject from 'dummy/tests/page-object';
 // only in the `moduleForComponent` test helper. Unfortunatelly it requires
 // some component name to be passed as a first argument. That's why we pass
 // a `calculating-device` here despite the fact that we don't actually need it.
-moduleForComponent('calculating-device', 'Integration | comma separated selectors', {
+moduleForComponent('calculating-device', 'Deprecation | comma separated selectors', {
   integration: true
 });
 

tests/integration/deprecations/legacy-collection-test.js

@@ -0,0 +1,38 @@
+import { moduleForComponent, test } from 'ember-qunit';
+
+import { create, collection } from 'ember-cli-page-object';
+
+moduleForComponent('calculating-device', 'Deprecation | legacy collection', {
+  integration: true
+});
+
+test('shows deprecation warning when first parameter is not a string', function(assert) {
+  let page = create({
+    foo: collection({
+      itemScope: 'li'
+    })
+  });
+
+  page.foo();
+
+  assert.expectDeprecation('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.');
+});
+
+test("doesn't show a deprecation warning when first parameter is a string", function(assert) {
+  let page = create({
+    foo: collection('foo')
+  });
+
+  page.foo;
+
+  assert.expectNoDeprecation();
+});
+
+test('shows a warning on invalid legacy collection definitions', function(assert) {
+  assert.expectWarning(function() {
+    create({
+      foo: collection({
+      })
+    });
+  }, 'Legacy page object collection definition is invalid. Please, make sure you include a `itemScope` selector.');
+});