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.

Sign up for GitHub

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

Jump to bottom

Comments to vue #23173

Merged
merged 4 commits into from
Oct 20, 2020
Merged

Comments to vue #23173

merged 4 commits into from
Oct 20, 2020

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Oct 4, 2020 •
edited
Loading

Ref #20020

How to use the comments app in your app

  1. Load the comments app 
    use OCP\Comments\ICommentsManager;

/** @var ICommentsManager */
private $commentsManager;

$this->commentsManager->load()
  2. Create a new instance 
    const commentsView = new OCA.Comments.View('announcement')
await commentsView.update(ressourceId)
commentsView.$mount(el)
  3. Profit

Cleanups

I removed all the tab leftovers, the only thing left is the filesPlugin (inline comment count)

@npmbuildbot-nextcloud npmbuildbot-nextcloud bot force-pushed the feat/files-sidebar-cleanup-standards branch from df5bae2 to 4fafd65 Compare October 5, 2020 10:09
@skjnldsv skjnldsv force-pushed the feat/files-sidebar-cleanup-standards branch 2 times, most recently from 092103d to 185f844 Compare October 7, 2020 07:41
@npmbuildbot-nextcloud npmbuildbot-nextcloud bot force-pushed the feat/files-sidebar-cleanup-standards branch from 185f844 to eca4682 Compare October 7, 2020 07:53
Base automatically changed from feat/files-sidebar-cleanup-standards to master October 7, 2020 09:13
@skjnldsv skjnldsv added this to the Nextcloud 21 milestone Oct 8, 2020
@skjnldsv skjnldsv mentioned this pull request Oct 8, 2020
Closed
4 tasks
@skjnldsv skjnldsv changed the title Init vue comments tab Comments to vue Oct 8, 2020
@nextcloud nextcloud deleted a comment from faily-bot bot Oct 8, 2020
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 16, 2020
@skjnldsv skjnldsv marked this pull request as ready for review October 16, 2020 20:53
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 16, 2020
@skjnldsv
Copy link
Member Author

I was too fast, I forgot something for the mentions. Feel free to test anyway, there will most likely be some other changes to do!
Sorry for the premature noise! 😞

@skjnldsv skjnldsv mentioned this pull request Oct 17, 2020
Closed
@skjnldsv

This comment has been minimized.

Sign in to view
@skjnldsv
Add ICommentsManager::load method
3e19160 
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
@skjnldsv
Copy link
Member Author

/compile amend /

ChristophWurst
ChristophWurst reviewed Oct 20, 2020
View reviewed changes
package.json
@@ -39,12 +39,13 @@
"@nextcloud/password-confirmation": "^1.0.1",
"@nextcloud/paths": "^1.1.2",
"@nextcloud/router": "^1.1.0",
"@nextcloud/vue": "^2.7.0",
"@nextcloud/vue": "^3.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

sneaky

Copy link
Member

Choose a reason for hiding this comment

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

🐍-y

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it?
Capture d’écran_2020-10-20_14-54-48

Copy link
Member

Choose a reason for hiding this comment

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

yes

Copy link
Member Author

Choose a reason for hiding this comment

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

IS IT?
Capture d’écran_2020-10-20_15-09-11

Copy link
Member

Choose a reason for hiding this comment

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

for the event of a Vue upgrade in Nextcloud I expect a cake delivered to my home so we can celebrate properly

@skjnldsv
Copy link
Member Author

Only took 16 minutes xD
Capture d’écran_2020-10-20_15-08-09

@skjnldsv
Copy link
Member Author

Raaah, missing a lib update, final push™

@skjnldsv
Copy link
Member Author

Tests needs fixing, but the tests looks awful 😢

rullzer
rullzer previously requested changes Oct 20, 2020
View reviewed changes
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.

Pressing enter doesn't submit the comment

@skjnldsv
Copy link
Member Author

Tests needs fixing, but the tests looks awful

Fixed

Pressing enter doesn't submit the comment

Fixed

@skjnldsv skjnldsv requested a review from rullzer October 20, 2020 14:26
skjnldsv
skjnldsv commented Oct 20, 2020
View reviewed changes
lib/private/Collaboration/Collaborators/UserPlugin.php Outdated
@@ -178,6 +180,8 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) {
if ($addToWideResults) {
$result['wide'][] = [
'label' => $userDisplayName,
'subline' => $status['message'],
Copy link
Member Author

@skjnldsv skjnldsv Oct 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

php 7.3 NODB tests are screaming

- Test\Collaboration\Collaborators\UserPluginTest::testSearch with data set #4 ('test', false, true, array(), array(), array(array('Test', array(0, 'test'), 'icon-user', null, array())), array(), true, Mock_IUser_27cf199a Object (...))
- Undefined index: message

What is the best way to make sure we have 'message' here @ChristophWurst @rullzer ?

Copy link
Member

Choose a reason for hiding this comment

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

what to you want to ensure? that it's always passed? hard to enforce with the array parameter.

how about a fallback with 'subline' => $status['message'] ?? ''?

@skjnldsv
Bump @nextcloud/vue to 3.0.0
6cd7549 
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
Signed-off-by: npmbuildbot[bot] <npmbuildbot[bot]@users.noreply.github.com>
@faily-bot
Copy link

faily-bot bot commented Oct 20, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 34389: failure

integration-sharing-v1-video-verification

  • build/integration/sharing_features/sharing-v1-video-verification.feature:8
Show full log 
  Scenario: Creating a link share with send password by Talk # /drone/src/build/integration/sharing_features/sharing-v1-video-verification.feature:8
[Tue Oct 20 17:17:00 2020] ArgumentCountError: Too few arguments to function OC\Comments\Manager::__construct(), 3 passed in /drone/src/apps/spreed/lib/Chat/CommentsManager.php on line 46 and exactly 4 expected at /drone/src/lib/private/Comments/Manager.php#73
[Tue Oct 20 17:17:00 2020] 127.0.0.1:46422 [200]: /ocs/v2.php/cloud/users/user0
[Tue Oct 20 17:17:00 2020] ArgumentCountError: Too few arguments to function OC\Comments\Manager::__construct(), 3 passed in /drone/src/apps/spreed/lib/Chat/CommentsManager.php on line 46 and exactly 4 expected at /drone/src/lib/private/Comments/Manager.php#73
[Tue Oct 20 17:17:00 2020] 127.0.0.1:46470 [200]: /ocs/v2.php/cloud/users/user0
    Given user "user0" exists                                # SharingContext::assureUserExists()
    And As an "user0"                                        # SharingContext::asAn()
[Tue Oct 20 17:17:00 2020] Login failed: 'user0' (Remote IP: '127.0.0.1')
[Tue Oct 20 17:17:00 2020] 127.0.0.1:46484 [401]: /ocs/v1.php/apps/files_sharing/api/v1/shares
    When creating a share with                               # SharingContext::creatingShare()
      | path               | welcome.txt |
      | shareType          | 3           |
      | password           | secret      |
      | sendPasswordByTalk | true        |
    Then the OCS status code should be "100"                 # SharingContext::theOCSStatusCodeShouldBe()
      Failed asserting that SimpleXMLElement Object &0000000046d06c9b0000000009181250 (
          0 => '997'
      ) matches expected '100'.
    And the HTTP status code should be "200"                 # SharingContext::theHTTPStatusCodeShouldBe()
    And last share with password "secret" can be downloaded  # SharingContext::lastShareWithPasswordCanBeDownloaded()

@skjnldsv
Copy link
Member Author 

[Tue Oct 20 17:17:00 2020] ArgumentCountError: Too few arguments to function OC\Comments\Manager::__construct(), 3 passed in /drone/src/apps/spreed/lib/Chat/CommentsManager.php on line 46 and exactly 4 expected at /drone/src/lib/private/Comments/Manager.php#73
[Tue Oct 20 17:17:00 2020] 127.0.0.1:46422 [200]: /ocs/v2.php/cloud/users/user0
[Tue Oct 20 17:17:00 2020] ArgumentCountError: Too few arguments to function OC\Comments\Manager::__construct(), 3 passed in /drone/src/apps/spreed/lib/Chat/CommentsManager.php on line 46 and exactly 4 expected at /drone/src/lib/private/Comments/Manager.php#73

Tests are failing on spreed, require a fix there

@skjnldsv skjnldsv merged commit 766590d into master Oct 20, 2020
@skjnldsv skjnldsv deleted the feat/comments-sidebar-vue branch October 20, 2020 18:46
@skjnldsv skjnldsv mentioned this pull request Oct 21, 2020
Merged
@MorrisJobke MorrisJobke mentioned this pull request Nov 3, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@rullzer rullzer rullzer approved these changes

@kesselb kesselb Awaiting requested review from kesselb

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@PVince81 PVince81 Awaiting requested review from PVince81

@julien-nc julien-nc Awaiting requested review from julien-nc

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement feature: comments
Projects
None yet
Milestone
Nextcloud 21
Development

Successfully merging this pull request may close these issues.
4 participants
@skjnldsv @rullzer @ChristophWurst @kesselb