Skip to content

Conversation

@miaulalala
Copy link
Contributor

@miaulalala miaulalala commented Jan 22, 2026

☑️ Resolves

Fixes #16777

🛠️ API Checklist

🚧 Tasks

  • ...

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@nickvergessen nickvergessen added this to the 🏖️ Next Major (34) milestone Jan 22, 2026
@miaulalala miaulalala added 3. to review bug feature: chat 💬 Chat and system messages feature: api 🛠️ OCS API for conversations, chats and participants labels Jan 26, 2026
@miaulalala miaulalala marked this pull request as ready for review January 26, 2026 12:32
@miaulalala miaulalala force-pushed the fix/16777/add-metadata branch from dc37a9a to 4af0d13 Compare January 26, 2026 12:43
@nickvergessen
Copy link
Member

Can we add a test?

@nickvergessen
Copy link
Member

/backport to stable33

* createdAt: int,
* sendAt: int,
* silent: bool,
* originalSendAt?: int,
Copy link
Member

Choose a reason for hiding this comment

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

Should add inline docs when this is the case and on sendAt document when it could be 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should add inline docs when this is the case and on sendAt document when it could be 0

What does that look like? Add it on the toArray() Method?

Copy link
Member

Choose a reason for hiding this comment

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

* // The unique identifier for the given actor type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* // The unique identifier for the given actor type

Neat, thanks, I wasn't sure what was meant with "Inline docs" 🙈

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Otherwise works as expected, thanks)

Image

@miaulalala
Copy link
Contributor Author

Can we add a test?

I added the data to the current test setup, it will run any time sendAt is 0

@nickvergessen
Copy link
Member

I added the data to the current test setup, it will run any time sendAt is 0

There was no integration test failing before.

IIRC potential test would be:

  1. basic room setup
  2. schedule a message
  3. revoke chat permission from user in 2
  4. run background job
  5. as user from 2 check your schedules?

@miaulalala
Copy link
Contributor Author

I added the data to the current test setup, it will run any time sendAt is 0

There was no integration test failing before.

IIRC potential test would be:

1. basic room setup

2. schedule a message

3. revoke chat permission from user in 2

4. run background job

5. as user from 2 check your schedules?

Literally

Scenario: Schedule a reply, but the account's chat permissions have been revoked in the meantime

@nickvergessen
Copy link
Member

Right, then we can add originalSendAt there on this lines?

    Then user "participant2" sees the following scheduled messages in room "room" with 200
      | id        | actorType | actorId      | threadId | parent | message   | messageType | sendAt | silent |
      | Message 1 | users     | participant2 | 0        | null   | Message 1 | comment     | 0      | false

That basically would test your PR

@miaulalala
Copy link
Contributor Author

Right, then we can add originalSendAt there on this lines?

    Then user "participant2" sees the following scheduled messages in room "room" with 200
      | id        | actorType | actorId      | threadId | parent | message   | messageType | sendAt | silent |
      | Message 1 | users     | participant2 | 0        | null   | Message 1 | comment     | 0      | false

That basically would test your PR

Why, it's already being added in the FeatureContext:

if ($row['sendAt'] !== '0') {
				$row['sendAt'] = self::$messageIdToTimestamp[$row['id']];
			} else {
				$row['originalSendAt'] = self::$messageIdToTimestamp[$row['id']];
			}

@nickvergessen
Copy link
Member

Ah, now I see it.
instead of having the time in the test itself, you add the value magically with the FeatureContext

@nickvergessen
Copy link
Member

I added a small commit to make the test more clear, let me know what you think

@miaulalala
Copy link
Contributor Author

I added a small commit to make the test more clear, let me know what you think

Looks good, thanks for that! Shall I squash and rebase?

@nickvergessen
Copy link
Member

After you added the docs you can do that yeah

@miaulalala miaulalala force-pushed the fix/16777/add-metadata branch from 0120384 to 07f50ba Compare January 27, 2026 08:32
miaulalala and others added 2 commits January 27, 2026 09:39
[skip-ci]

Signed-off-by: Anna Larch <anna@nextcloud.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@miaulalala miaulalala force-pushed the fix/16777/add-metadata branch from cb0f258 to 06a9cb7 Compare January 27, 2026 08:39
@nickvergessen nickvergessen merged commit 39c69d9 into main Jan 27, 2026
96 of 109 checks passed
@nickvergessen nickvergessen deleted the fix/16777/add-metadata branch January 27, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug feature: api 🛠️ OCS API for conversations, chats and participants feature: chat 💬 Chat and system messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed scheduled messages do not have original sendAt parameter anywhere as well as error message

4 participants