Skip to content

Inidicate preview availability in share api responses #23350

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
Merged
MorrisJobke merged 1 commit into from
Oct 13, 2020
Merged

Inidicate preview availability in share api responses #23350

MorrisJobke merged 1 commit into from
Oct 13, 2020

Conversation

@st3iny
Copy link
Member

@st3iny st3iny commented Oct 11, 2020

Fixes #14849

The share api does not indicate if a preview is available for a share. This causes data-has-preview="true" for every entry in the share file list. The border around items without a preview is a consequence of this missing indication.

@st3iny
Copy link
Member Author

st3iny commented Oct 12, 2020

@juliushaertl This PR is ready to be reviewed.

juliusknorr
juliusknorr approved these changes Oct 12, 2020
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Nice catch 👍

@juliusknorr juliusknorr added 3. to review Waiting for reviews bug labels Oct 12, 2020
@juliusknorr juliusknorr added this to the Nextcloud 21 milestone Oct 12, 2020
@juliusknorr
Copy link
Member

juliusknorr commented Oct 12, 2020

/backport to stable20

@juliusknorr
Copy link
Member

juliusknorr commented Oct 12, 2020

/backport to stable19

@juliusknorr
Copy link
Member

juliusknorr commented Oct 12, 2020

/backport to stable18

@skjnldsv
Copy link
Member

skjnldsv commented Oct 12, 2020

Nice!!

@skjnldsv
Copy link
Member

skjnldsv commented Oct 12, 2020

Please rebase :)

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 12, 2020
@MorrisJobke
Copy link
Member

MorrisJobke commented Oct 12, 2020

Please rebase :)

Done

@MorrisJobke
Copy link
Member

MorrisJobke commented Oct 12, 2020

/compile /

@skjnldsv

This comment has been minimized.

Sign in to view
@skjnldsv

This comment has been minimized.

Sign in to view
@skjnldsv
Copy link
Member

skjnldsv commented Oct 12, 2020

Oh right, no conflicts because that is only hanging apps/files_sharing/js/dist/files_sharing.js
Cool! LGTM then! 🚀

@MorrisJobke
Copy link
Member

MorrisJobke commented Oct 12, 2020

PHUnit tests are broken:

There were 124 errors:
94	
95	1) OCA\Files_Sharing\Tests\Controller\ShareAPIControllerTest::testDeleteShareShareNotFound
96	ArgumentCountError: Too few arguments to function OCA\Files_Sharing\Controller\ShareAPIController::__construct(), 13 passed in /drone/src/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php on line 151 and exactly 14 expected

@st3iny
Copy link
Member Author

st3iny commented Oct 12, 2020

Oops ... Just saw the tests. I'm on it but fixing this might take some time.

@skjnldsv
Copy link
Member

skjnldsv commented Oct 12, 2020

but fixing this might take some time.

Take all you need, there is no rush :)

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Oct 12, 2020
@st3iny
Copy link
Member Author

st3iny commented Oct 13, 2020

I fixed all the existing tests by always expecting no preview and added an new test data set to ShareApiControllerTest::testFormatShare to test a file with a preview available (should return has_preview=true).

@skjnldsv
Copy link
Member

skjnldsv commented Oct 13, 2020

Lint / php-cs check is failing

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Oct 13, 2020
@MorrisJobke MorrisJobke merged commit 7525fb3 into nextcloud:master Oct 13, 2020
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Oct 13, 2020
Merged
@MorrisJobke
Copy link
Member

MorrisJobke commented Nov 3, 2020

/backport to stable19

@MorrisJobke
Copy link
Member

MorrisJobke commented Nov 3, 2020

/backport to stable18

This was referenced Nov 3, 2020
Merged
Merged
@st3iny st3iny deleted the fix/14849/share-api-preview-availability branch November 7, 2020 00:40
@st3iny st3iny mentioned this pull request Dec 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@juliusknorr juliusknorr juliusknorr approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish bug

Projects

None yet

Milestone

Nextcloud 21

Development

Successfully merging this pull request may close these issues.

Border around mime icon in file list

4 participants

@st3iny @juliusknorr @skjnldsv @MorrisJobke