OfficeDev / Office-Addin-Scripts

A set of scripts and packages that are consumed in Office add-ins projects.
MIT License
160 stars 100 forks source link

OfficeMockObject cannot mock NamedItemCollection #729

Open HarryPi opened 1 year ago

HarryPi commented 1 year ago

Prerequisites

Please answer the following questions before submitting an issue. YOU MAY DELETE THE PREREQUISITES SECTION.

Expected behavior

Please describe the behavior you were expecting

I am trying to mock Named ranges array of excel so i can run some tests on adding and removing custom ranges, but it appears that the mock object is not equipped to handle arrays?

Using Jest and in Angular.

As per the documentation i am mocking the Host object and assigning it to the global.Excel

 it('should delete the first item', async () => {

    const workbookWithNamedRanges = {
      context: {
        workbook: {
          names: {
            items: [
              {
                name: 'Test 1',
                type: 'Error'
              },
              {
                name: 'Test 2',
                type: 'Error'
              },
            ]
          }
        }
      },
      run: async function (callback: any) {
        await callback(this.context);
      }
    };

    const excelMock = new OfficeMockObject(workbookWithNamedRanges) as any;

    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    global.Excel = excelMock;

    await service.service.removeFirstItem();

    expect(excelMock.context.workbook.names.items.length).toBe(1);
  });
});

Then in my service

async removeFirstItem(): Promise<void> {

    await Excel.run<void>(async context => {
      workbook = context.workbook;

      const namedRanges: Excel.NamedItemCollection = workbook.names.load() // <- Error here at .load;
      await context.sync();

      namedRanges.items.at(0)?.delete();

      await context.sync();

    });
}

The line mentioned above produces the following error

  Cannot convert undefined or null to object
TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)

Changing the code slightly to instead use context.load(workbook.names, 'items') produces a different error

Error: Property _properties needs to be present in object model before load is called.

    at C:\Users\User\Addins\addins\node_modules\office-addin-mock\src\officeMockObject.ts:168:15
    at Array.forEach (<anonymous>)
    at OfficeMockObject.parseObjectPropertyIntoArray (C:\Users\User\Addins\addins\node_modules\office-addin-mock\src\officeMockObject.ts:148:29)
    at OfficeMockObject.load (C:\Users\User\Addins\addins\node_modules\office-addin-mock\src\officeMockObject.ts:41:25)

Also trying workbook.names.load('items') works but items is undefined even though it was clearly defined in the mock object.

All of the ways tested above work when the code is run properly within an office environment and only fail with mocking.

The only workaround that worked was:

context.workbook.load('names')

Expected behaviour

I would expect any code that runs fine when not under test to work with the mock object as well.

millerds commented 1 year ago

Thanks for reporting this. It looks like there are 3 different issues with the mock package here.

ramzitannous commented 1 year ago

@HarryPi @millerds any workaround for this issue, this is happening for me also getting Error: Property values needs to be present in object model before load is called. while trying to mock this

  const items = [
      {
        columnIndex: 1,
        values: [[""]],
        formulas: [[""]],
        address: "Sheet1!B1",
      },
    ];
    const mock = new OfficeBddinMock.OfficeMockObject({
      areas: { items },
    });
    mock.load("areas/items/values");
    mock.sync();
    console.log(mock.areas.items[0].values);
millerds commented 1 year ago

@ramzitannous that is not the same thing. It's also incomplete . . . you aren't mocking the whole office-js path. On top of that I think your resulting json structure doesn't actually include an element named "items" . . . you just assigned a variable named items as part of an object named "areas" without giving it a property name in the object.

Look at https://learn.microsoft.com/en-us/office/dev/add-ins/testing/unit-testing for some usage examples.

ramzitannous commented 1 year ago

@millerds this pattern is needed for range area testing for ex. targetRangeArea.load("areas/items/values,areas/items/formulas,areas/items/address");

millerds commented 1 year ago

The original comment about collections is a currently limitation that needs to be addressed. Your error, however, is not the same thing even if you are trying to accomplish the same type of testing. Even if support for collects were there, you would be hitting your error because your mock structure above the collection isn't complete and the collection structure you are creating doesn't match the officejs collection structure (which is what you are trying to mock).

ramzitannous commented 1 year ago

@millerds the structure I'm mocking is exactly like what sheet.getRanges() returns, can you help and show what is wrong in the above code ?

millerds commented 1 year ago

OK . . . looks like I was mistaken. I thought that the way you were building the structure would leave an anonymous reference to the item collection, but apparently the variable name is keep and used at the property name. It's still just a fragment of the full api structure that would be used in the actual code. The error you getting is different because you're trying to load a property of one of the items of the collection which doesn't exist in the mock object created from the data object.

In any case . . . there isn't a workaround. The Mock object doesn't handle collections/arrays and so when parsing out the data input it stops going down at the array and moves onto the next property. This is a limitation of the current mock object. We have a work item on our back log to improve this.

ramzitannous commented 1 year ago

@millerds is there a timeline for implementing this ?

millerds commented 1 year ago

We don't comment on timelines.

asparrowhawk commented 3 weeks ago

It appears that the OfficeMockObject does not support any property that is an array.

For example:

const mockData = {
  context: {
    document: {
      body: {
        fields: {
          items: [
            {
              code: " DATE  \\* MERGEFORMAT ",
              result: '{"hyperlink":"","isEmpty":false,"style":"Normal","styleBuiltIn":"Normal","text":"23/10/2024"}',
            },
            {
              code: " SECTION  \\* MERGEFORMAT ",
              result: '{"hyperlink":"","isEmpty":false,"style":"Normal","styleBuiltIn":"Normal","text":"2"}',
            }
          ]
        },
      },
    },
  },
  run: async function (callback) {
    return await callback(this.context)
  },
}
const mock = new OfficeMockObject(mockData, OfficeApp.Word)

When the mock object is created, the field items indexed values are not themselves created as instances of OfficeMockObject. So when you look at mock.context.document.body.fields.items[0] in the debugger you can see it is just a plain object. Where as mock.context.document.body.fields is an instance of OfficeMockObject.

A solution that works for me is to use a dictionary like structure. For example:

const mockData = {
  context: {
    document: {
      body: {
        fields: {
          items: {
            0: {
              code: " DATE  \\* MERGEFORMAT ",
              result: '{"hyperlink":"","isEmpty":false,"style":"Normal","styleBuiltIn":"Normal","text":"23/10/2024"}',
            },
            1: {
              code: " SECTION  \\* MERGEFORMAT ",
              result: '{"hyperlink":"","isEmpty":false,"style":"Normal","styleBuiltIn":"Normal","text":"2"}',
            },
            length: 2,
          },
        },
      },
    },
  },
  run: async function (callback) {
    return await callback(this.context)
  },
}
const mock = new OfficeMockObject(mockData, OfficeApp.Word)

Now when accessing mock.context.document.body.fields.items[0] there will be an instance of OfficeMockObject with the expected code and result keys stored in its private _properties map.

The next problem is with the example javascript in the documentation, such as this https://learn.microsoft.com/en-us/javascript/api/word/word.fieldcollection?view=word-js-preview#examples. This code works with the real Office JS API and it will work in a unit test up until fields.load(["code", "result"]). To be able to access the the fields code and result property values with an instance of the OfficeMockObject you would need to use fields.load(["items/0/code", "items/0/result", "items/1/code", "items/1/result"]) instead.

One solution to the above problem is to "monkey patch" the OfficeMockObject:

if (!OfficeMockObject.prototype._load) {
  OfficeMockObject.prototype._load = OfficeMockObject.prototype.load
  OfficeMockObject.prototype.load = function (propertyArgument: string | string[] | ObjectData) {
    if (propertyArgument instanceof OfficeMockObject) {
      const mock = propertyArgument as OfficeMockObject
      return mock._load("*")
    } else if (
      typeof propertyArgument === "undefined" ||
      typeof propertyArgument === "string" ||
      Array.isArray(propertyArgument)
    ) {
      // access private fields of OfficeMockObject ...
      const mockItemsObj = { ...this["items"] }
      const properties = mockItemsObj._properties
      if (!properties) {
        // does not have an items property
        return this._load(propertyArgument)
      } else {
        const mockLengthObj = { ...properties.get("length") }
        const length = mockLengthObj._valueBeforeLoaded ? (mockLengthObj._valueBeforeLoaded as number) : 0
        if (typeof propertyArgument === "undefined") {
          return this._load("*")
        } else {
          const names = Array.isArray(propertyArgument)
            ? (propertyArgument as string[]).filter((name) => name.trim() !== "items")
            : propertyArgument?.split(",").filter((name) => name.trim() !== "items")
          const indices = [...Array(length).keys()]
          const paths = ["items"].concat(
            indices.flatMap((index) => names.map((name) => `items/${index}/${name.trim()}`)),
          )
          return this._load(paths)
        }
      }
    } else {
      return this._load(propertyArgument)
    }
  }
}

With the patch in place, the example code will now work as expected, with the fields.items.length, fields.items[i].code and fields.items[i].result returning the expected values.

Etschbeijer commented 3 weeks ago

Hi, we are working on an Excel addin that runs over a lot of tables and we want to write tests for it. We have the problem that we cannot access the collections.

When you have any information regarding this, we would be very curious about the development process.

Etschbeijer commented 3 weeks ago

@asparrowhawk Thank you very much, your monkey patch works fine for us.