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

Support copying the most recent item to the counter table #21

Closed
wants to merge 1 commit into from

Conversation

lpsinger
Copy link
Member

This is #20 as a one-liner.

@dakota002
Copy link
Contributor

dakota002 commented Oct 17, 2023

This does not achieve the same goals I had in mind on #20. How does this handle the case for circulars? How do you see this used for versioning?

In the case of an update to a specific circular, what kind of config options would you have to pass into this to result in keeping the same circularId and bumping the version field?

@lpsinger
Copy link
Member Author

In the case of an update to a specific circular, what kind of config options would you have to pass into this to result in keeping the same circularId and bumping the version field?

{
  doc,
  counterTableName: 'circulars',
  counterTableKey: {circularId: 33693 /* for example */},
  counterTableAttributeName: 'version',
  counterTableCopyItem: true,
  tableName: 'circulars_history',
  tableAttributeName: 'version',
  initialValue: 1,
}

@dakota002
Copy link
Contributor

dakota002 commented Oct 17, 2023

In the case of an update to a specific circular, what kind of config options would you have to pass into this to result in keeping the same circularId and bumping the version field?

{
  doc,
  counterTableName: 'circulars',
  counterTableKey: {circularId: 33693 /* for example */},
  counterTableAttributeName: 'version',
  counterTableCopyItem: true,
  tableName: 'circulars_history',
  tableAttributeName: 'version',
  initialValue: 1,
}

Ah, my use didn't swap the props around like this, I see how this could work now.

I think this is good, but there are a couple points that I think this misses:

  • As a user who is updating an existing circular, I would expect the result to be that I now have 2 results in the history table: Verisons 1 (the original) and 2 (the one with the edits). If this runs on a circular without an existing verison, it will create a new entry marked as version 1 in both tables, and the original is lost. Are we assuming that we should run a script or something that will backfill all existing circulars with a version: 1 property and populate the entire history table?
  • In the current tests, on adding a new item to the table, the counter table will overwrite the saved item, and only keep a copy of the latest inserted item. Is this resolved if the targeted counter table uses a sort key as well?

@lpsinger
Copy link
Member Author

Are we assuming that we should run a script or something that will backfill all existing circulars with a version: 1 property and populate the entire history table?

Correct.

  • In the current tests, on adding a new item to the table, the counter table will overwrite the saved item, and only keep a copy of the latest inserted item. Is this resolved if the targeted counter table uses a sort key as well?

I don't understand the question.

@dakota002
Copy link
Contributor

dakota002 commented Oct 17, 2023

  • In the current tests, on adding a new item to the table, the counter table will overwrite the saved item, and only keep a copy of the latest inserted item. Is this resolved if the targeted counter table uses a sort key as well?

I don't understand the question.

How do you maintain the history when inserting new rows?
I added the following test locally to check what happens when multiple items are intentionally inserted to the table:

test('maintains history when new items are inserted', async () => {
  const initialID = 1
  const firstResult = await autoincrement.put({
    widgetName: 'runcible spoon',
  })
  expect(firstResult).toEqual(initialID)
  expect(await autoincrement.getLast()).toEqual(initialID)

  const secondResult = await autoincrement.put({
    widgetName: 'a new widget',
  })
  expect(secondResult).toBe(initialID + 1)
  const [widgetItems, autoincrementItems] = await Promise.all(
    ['widgets', 'autoincrement'].map(
      async (TableName) => (await doc.scan({ TableName })).Items
    )
  )
  expect(widgetItems).toEqual([
    { widgetID: 2, widgetName: 'a new widget' },
    { widgetID: 1, widgetName: 'runcible spoon' },
  ])
  expect(autoincrementItems).toEqual(
    counterTableCopyItem
      ? [
          {
            tableName: 'widgets',
            counter: 1,
            widgetName: 'runcible spoon',
          },
          {
            tableName: 'widgets',
            counter: 2,
            widgetName: 'a new widget',
          },
        ]
      : [
          {
            tableName: 'widgets',
            counter: 2,
          },
        ]
  )
})

Which fails with the following when the copy flag is true (first object missing):

counterTableCopyItem=true › dynamoDBAutoIncrement › maintains history when new items are inserted

    expect(received).toEqual(expected) // deep equality

    - Expected  - 5
    + Received  + 0

      Array [
        Object {
    -     "counter": 1,
    -     "tableName": "widgets",
    -     "widgetName": "runcible spoon",
    -   },
    -   Object {
          "counter": 2,
          "tableName": "widgets",
          "widgetName": "a new widget",
        },
      ]

      153 |           { widgetID: 1, widgetName: 'runcible spoon' },       
      154 |         ])
    > 155 |         expect(autoincrementItems).toEqual(
          |                                    ^
      156 |           counterTableCopyItem
      157 |             ? [
      158 |                 {

      at Object.<anonymous> (src/test.ts:155:36)

@lpsinger
Copy link
Member Author

How do you maintain the history when inserting new rows?

Do you mean: how do you maintain history when inserting new GCN Circulars (entirely new Circulars that don't have a previous version)?

@dakota002
Copy link
Contributor

How do you maintain the history when inserting new rows?

Do you mean: how do you maintain history when inserting new GCN Circulars (entirely new Circulars that don't have a previous version)?

Yes

@lpsinger
Copy link
Member Author

How do you maintain the history when inserting new rows?

Do you mean: how do you maintain history when inserting new GCN Circulars (entirely new Circulars that don't have a previous version)?

Yes

Good point. I have to think about that.

@lpsinger
Copy link
Member Author

Closing in favor of #20.

@lpsinger lpsinger closed this Oct 31, 2023
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