Skip to content

Cleanup sharing leftovers - #17753

Merged
rullzer merged 2 commits into
masterfrom
clean/old-sharing
Nov 6, 2019
Merged

Cleanup sharing leftovers#17753
rullzer merged 2 commits into
masterfrom
clean/old-sharing

Conversation

@skjnldsv

@skjnldsv skjnldsv commented Oct 30, 2019

Copy link
Copy Markdown
Member

Signed-off-by: John Molakvoæ (skjnldsv) skjnldsv@protonmail.com

@skjnldsv skjnldsv added 3. to review Waiting for reviews feature: sharing feature: files feature: file sidebar Related to the file sidebar component labels Oct 30, 2019
@skjnldsv skjnldsv added this to the Nextcloud 18 milestone Oct 30, 2019
@skjnldsv skjnldsv self-assigned this Oct 30, 2019
skjnldsv
skjnldsv commented Oct 30, 2019
View reviewed changes
Comment thread apps/files_sharing/src/share.js
* @param hasShares whether shares are available
* @param hasLink whether link share is available
*/
_markFileAsShared: function($tr, hasShares, hasLink) {

@skjnldsv skjnldsv Oct 30, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it seemed to be the only function used from the core/js/share.js, so I moved it here to nuke sharing from core! 💥 🧹 🧼

skjnldsv
skjnldsv commented Oct 30, 2019
View reviewed changes
Comment thread apps/files_sharing/src/utils/SharedWithMe.js
*/
const shareWithTitle = function(share) {
if (share.type === OC.Share.type_GROUP) {
if (share.type === OC.Share.SHARE_TYPE_GROUP) {

@skjnldsv skjnldsv Oct 30, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Somehow I missed this 🤷‍♂️

@skjnldsv
skjnldsv requested a review from juliusknorr October 30, 2019 14:46
@skjnldsv skjnldsv mentioned this pull request Oct 30, 2019
Closed
14 tasks
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 30, 2019
@skjnldsv

skjnldsv commented Oct 30, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

I forgot some scripts and I'm not really happy with how it's design, especially the way we declare th OC.Sharing.SHARE_TYPE_XXX constants.

So, shares types are declaring the selves (in js) in here

server/lib/private/Share/Share.php

Lines 87 to 89 in 938428c

if(count(self::$backendTypes) === 1) {
Util::addScript('core', 'dist/share_backend');
}
which is quite ugly !

server/core/js/share.js

Lines 5 to 15 in 938428c

OC.Share = _.extend(OC.Share || {}, {
SHARE_TYPE_USER: 0,
SHARE_TYPE_GROUP: 1,
SHARE_TYPE_LINK: 3,
SHARE_TYPE_EMAIL: 4,
SHARE_TYPE_REMOTE: 6,
SHARE_TYPE_CIRCLE: 7,
SHARE_TYPE_GUEST: 8,
SHARE_TYPE_REMOTE_GROUP: 9,
SHARE_TYPE_ROOM: 10,

Ideally I would like not to have a dedicated script http request to fetch this
I think it should be included in all pages, but in the page data
How should we do this? Do you have another idea?
(or was the old method ok?)

ChristophWurst
ChristophWurst approved these changes Nov 4, 2019
View reviewed changes

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐘

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 4, 2019
@skjnldsv
Cleanup sharing leftovers
eebcf89 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Fix sharing breadcrumbs
5d2c89c 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
rullzer
rullzer approved these changes Nov 6, 2019
View reviewed changes

@rullzer rullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did some smoke testing. Didn't do 💥
Lets do this.

@rullzer
rullzer merged commit 3a3ed95 into master Nov 6, 2019
@rullzer
rullzer deleted the clean/old-sharing branch November 6, 2019 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzer rullzer rullzer approved these changes
@ChristophWurst ChristophWurst ChristophWurst approved these changes
@georgehrke georgehrke Awaiting requested review from georgehrke
@kesselb kesselb Awaiting requested review from kesselb
@danxuliu danxuliu Awaiting requested review from danxuliu
@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees

@skjnldsv skjnldsv

Labels

3. to review Waiting for reviews feature: file sidebar Related to the file sidebar component feature: files feature: sharing

Projects

None yet

Milestone

Nextcloud 18.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@skjnldsv @rullzer @ChristophWurst