Skip to content

Conversation

@nickvergessen
Copy link
Member

So for talks use of the comments api, we would be interested in an additional field for comments to be added. We would store there a random hash, so when the server messages are returned while you e.g. post a message we can see if the message is the one you are currently sending and already hide the temporary message.
You can currently see this sometimes when a lot of notifications are sent, that the message is shown twice for some milliseconds, because currently the temporary message is killed by the response of the post (which happens after the notifications are sent to the proxy) but in parallel the message appears already from the "waiting for new messages" request immediately after its written to the database.

So this PR adds the optional column, and only writes/updates it when the column was added already. This laziness is okay, as the impact is sloppier behaviour instead of broken behaviour in case the column was not added.

protected function hasMissingColumns(): array {
$indexInfo = new MissingColumnInformation();
// Dispatch event so apps can also hint for pending index updates if needed
$event = new GenericEvent($indexInfo);
Copy link
Member Author

Choose a reason for hiding this comment

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

For now i copied the indexes code, but we could also go for a new event class here making things proper. i dont care too much

Copy link
Member

Choose a reason for hiding this comment

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

Depends. If we consider this public then I would go for the new things straight away

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, fine by me

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Fine by me

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the enh/comments-reference-id branch from 15dae1b to 60d9384 Compare March 31, 2020 08:51
@nickvergessen
Copy link
Member Author

Squash time

@nickvergessen nickvergessen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 31, 2020
@blizzz
Copy link
Member

blizzz commented Mar 31, 2020

…
PhantomJS  2.1.1 (Linux 0.0.0) OC.SetupChecks tests checkSetup should return an  error if the protocol is https but the server generates http links  FAILED
TypeError: undefined is not an object (evaluating 'data.missingColumns.length') in core/js/setupchecks.js (line 1) 
afterCall@core/js/setupchecks.js:1:80437 
…

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member Author

Updated the one new test as well

@blizzz
Copy link
Member

blizzz commented Apr 1, 2020

There's more:

…
There was 1 failure:
1) OCA\DAV\Tests\unit\Comments\CommentsNodeTest::testGetProperties 
Failed asserting that false is true. 

/drone/src/apps/dav/tests/unit/Comments/CommentsNodeTest.php:487 

i didn't check all steps.

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen merged commit bc6a5ef into master Apr 2, 2020
@nickvergessen nickvergessen deleted the enh/comments-reference-id branch April 2, 2020 09:34
ChristophWurst added a commit to nextcloud/documentation that referenced this pull request Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish enhancement feature: comments technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants