Skip to content

Commit 642d623

Browse files
[POC] [WIP] refactor(files_sharing): move getSuggestions(), getRecommendations() out
I like to reach the goals, but the new state show's how entangled the code is. Let's me think about this change - I might park or drop this commit. Otherwise: let's discuss. == Goal * Make suggestion/recommendation code reusable * Have it as separate module * to reduce LOC of SharingInput * to make the extracted function easier testable == Notable changes Apart from obvious moving code: 1. this.externalResults is actually OCA.Sharing.ShareSearch.state.results This is now directly referenced. 2. the "lookup" parameter to getSuggestions() was never passed and internally defaulted The parameter was therefor removed. == Findings This reveals a bit of a mess: 1. the filter function depends heavily on the Vue context 2. the formatting of API results for UI display is mixed with combining data 3. external results (OCA.Sharing.ShareSearch.state) are already formatted == Refactoring left-overs 1. existingSharesFilter() depends on reshare, linkShares, shares from the Vue object I'd like to keep this function independant from Vue actually, but stop here to not go deeper into a refactoring rabbit hole. Marked as TODO. 2. Separation of data fetching and data combining from formatting.
1 parent 2fb95df commit 642d623

File tree

5 files changed

+304
-265
lines changed

5 files changed

+304
-265
lines changed

apps/files_sharing/src/components/SharingInput.vue

+9-265
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ import ShareRequests from '../mixins/ShareRequests.js'
4040
import ShareDetails from '../mixins/ShareDetails.js'
4141
import { ShareType } from '@nextcloud/sharing'
4242

43+
import getRecommendations from '../services/recommendations.js';
44+
import getSuggestions from '../services/suggestions.js';
45+
4346
export default {
4447
name: 'SharingInput',
4548

@@ -81,24 +84,12 @@ export default {
8184
loading: false,
8285
query: '',
8386
recommendations: [],
84-
ShareSearch: OCA.Sharing.ShareSearch.state,
8587
suggestions: [],
8688
value: null,
8789
}
8890
},
8991

9092
computed: {
91-
/**
92-
* Implement ShareSearch
93-
* allows external appas to inject new
94-
* results into the autocomplete dropdown
95-
* Used for the guests app
96-
*
97-
* @return {Array}
98-
*/
99-
externalResults() {
100-
return this.ShareSearch.results
101-
},
10293
inputPlaceholder() {
10394
const allowRemoteSharing = this.config.isRemoteShareAllowed
10495

@@ -158,105 +149,18 @@ export default {
158149
* Get suggestions
159150
*
160151
* @param {string} search the search query
161-
* @param {boolean} [lookup] search on lookup server
162152
*/
163-
async getSuggestions(search, lookup = false) {
153+
async getSuggestions(search) {
164154
this.loading = true
165155

166-
if (getCapabilities().files_sharing.sharee.query_lookup_default === true) {
167-
lookup = true
168-
}
169-
170-
const shareType = [
171-
ShareType.User,
172-
ShareType.Group,
173-
ShareType.Remote,
174-
ShareType.RemoteGroup,
175-
ShareType.Team,
176-
ShareType.Room,
177-
ShareType.Guest,
178-
ShareType.Deck,
179-
ShareType.ScienceMesh,
180-
]
181-
182-
if (getCapabilities().files_sharing.public.enabled === true) {
183-
shareType.push(ShareType.Email)
184-
}
185-
186-
let request = null
187156
try {
188-
request = await axios.get(generateOcsUrl('apps/files_sharing/api/v1/sharees'), {
189-
params: {
190-
format: 'json',
191-
itemType: this.fileInfo.type === 'dir' ? 'folder' : 'file',
192-
search,
193-
lookup,
194-
perPage: this.config.maxAutocompleteResults,
195-
shareType,
196-
},
197-
})
157+
this.suggestions = await getSuggestions(search, this.fileInfo, this, this.config)
158+
this.loading = false
159+
console.info('suggestions', this.suggestions)
198160
} catch (error) {
199161
console.error('Error fetching suggestions', error)
200162
return
201163
}
202-
203-
const data = request.data.ocs.data
204-
const exact = request.data.ocs.data.exact
205-
data.exact = [] // removing exact from general results
206-
207-
// flatten array of arrays
208-
const rawExactSuggestions = Object.values(exact).reduce((arr, elem) => arr.concat(elem), [])
209-
const rawSuggestions = Object.values(data).reduce((arr, elem) => arr.concat(elem), [])
210-
211-
// remove invalid data and format to user-select layout
212-
const exactSuggestions = this.filterOutExistingShares(rawExactSuggestions)
213-
.map(share => this.formatForMultiselect(share))
214-
// sort by type so we can get user&groups first...
215-
.sort((a, b) => a.shareType - b.shareType)
216-
const suggestions = this.filterOutExistingShares(rawSuggestions)
217-
.map(share => this.formatForMultiselect(share))
218-
// sort by type so we can get user&groups first...
219-
.sort((a, b) => a.shareType - b.shareType)
220-
221-
// lookup clickable entry
222-
// show if enabled and not already requested
223-
const lookupEntry = []
224-
if (data.lookupEnabled && !lookup) {
225-
lookupEntry.push({
226-
id: 'global-lookup',
227-
isNoUser: true,
228-
displayName: t('files_sharing', 'Search globally'),
229-
lookup: true,
230-
})
231-
}
232-
233-
// if there is a condition specified, filter it
234-
const externalResults = this.externalResults.filter(result => !result.condition || result.condition(this))
235-
236-
const allSuggestions = exactSuggestions.concat(suggestions).concat(externalResults).concat(lookupEntry)
237-
238-
// Count occurrences of display names in order to provide a distinguishable description if needed
239-
const nameCounts = allSuggestions.reduce((nameCounts, result) => {
240-
if (!result.displayName) {
241-
return nameCounts
242-
}
243-
if (!nameCounts[result.displayName]) {
244-
nameCounts[result.displayName] = 0
245-
}
246-
nameCounts[result.displayName]++
247-
return nameCounts
248-
}, {})
249-
250-
this.suggestions = allSuggestions.map(item => {
251-
// Make sure that items with duplicate displayName get the shareWith applied as a description
252-
if (nameCounts[item.displayName] > 1 && !item.desc) {
253-
return { ...item, desc: item.shareWithDisplayNameUnique }
254-
}
255-
return item
256-
})
257-
258-
this.loading = false
259-
console.info('suggestions', this.suggestions)
260164
},
261165

262166
/**
@@ -274,175 +178,15 @@ export default {
274178
async getRecommendations() {
275179
this.loading = true
276180

277-
let request = null
278181
try {
279-
request = await axios.get(generateOcsUrl('apps/files_sharing/api/v1/sharees_recommended'), {
280-
params: {
281-
format: 'json',
282-
itemType: this.fileInfo.type,
283-
},
284-
})
182+
this.recommendations = await getRecommendations(this.fileInfo, this, this.config);
183+
console.info('recommendations', this.recommendations)
285184
} catch (error) {
286185
console.error('Error fetching recommendations', error)
287186
return
288187
}
289188

290-
// Add external results from the OCA.Sharing.ShareSearch api
291-
const externalResults = this.externalResults.filter(result => !result.condition || result.condition(this))
292-
293-
// flatten array of arrays
294-
const rawRecommendations = Object.values(request.data.ocs.data.exact)
295-
.reduce((arr, elem) => arr.concat(elem), [])
296-
297-
// remove invalid data and format to user-select layout
298-
this.recommendations = this.filterOutExistingShares(rawRecommendations)
299-
.map(share => this.formatForMultiselect(share))
300-
.concat(externalResults)
301-
302189
this.loading = false
303-
console.info('recommendations', this.recommendations)
304-
},
305-
306-
/**
307-
* Filter out existing shares from
308-
* the provided shares search results
309-
*
310-
* @param {object[]} shares the array of shares object
311-
* @return {object[]}
312-
*/
313-
filterOutExistingShares(shares) {
314-
return shares.reduce((arr, share) => {
315-
// only check proper objects
316-
if (typeof share !== 'object') {
317-
return arr
318-
}
319-
try {
320-
if (share.value.shareType === ShareType.User) {
321-
// filter out current user
322-
if (share.value.shareWith === getCurrentUser().uid) {
323-
return arr
324-
}
325-
326-
// filter out the owner of the share
327-
if (this.reshare && share.value.shareWith === this.reshare.owner) {
328-
return arr
329-
}
330-
}
331-
332-
// filter out existing mail shares
333-
if (share.value.shareType === ShareType.Email) {
334-
const emails = this.linkShares.map(elem => elem.shareWith)
335-
if (emails.indexOf(share.value.shareWith.trim()) !== -1) {
336-
return arr
337-
}
338-
} else { // filter out existing shares
339-
// creating an object of uid => type
340-
const sharesObj = this.shares.reduce((obj, elem) => {
341-
obj[elem.shareWith] = elem.type
342-
return obj
343-
}, {})
344-
345-
// if shareWith is the same and the share type too, ignore it
346-
const key = share.value.shareWith.trim()
347-
if (key in sharesObj
348-
&& sharesObj[key] === share.value.shareType) {
349-
return arr
350-
}
351-
}
352-
353-
// ALL GOOD
354-
// let's add the suggestion
355-
arr.push(share)
356-
} catch {
357-
return arr
358-
}
359-
return arr
360-
}, [])
361-
},
362-
363-
/**
364-
* Get the icon based on the share type
365-
*
366-
* @param {number} type the share type
367-
* @return {string} the icon class
368-
*/
369-
shareTypeToIcon(type) {
370-
switch (type) {
371-
case ShareType.Guest:
372-
// default is a user, other icons are here to differentiate
373-
// themselves from it, so let's not display the user icon
374-
// case ShareType.Remote:
375-
// case ShareType.User:
376-
return {
377-
icon: 'icon-user',
378-
iconTitle: t('files_sharing', 'Guest'),
379-
}
380-
case ShareType.RemoteGroup:
381-
case ShareType.Group:
382-
return {
383-
icon: 'icon-group',
384-
iconTitle: t('files_sharing', 'Group'),
385-
}
386-
case ShareType.Email:
387-
return {
388-
icon: 'icon-mail',
389-
iconTitle: t('files_sharing', 'Email'),
390-
}
391-
case ShareType.Team:
392-
return {
393-
icon: 'icon-teams',
394-
iconTitle: t('files_sharing', 'Team'),
395-
}
396-
case ShareType.Room:
397-
return {
398-
icon: 'icon-room',
399-
iconTitle: t('files_sharing', 'Talk conversation'),
400-
}
401-
case ShareType.Deck:
402-
return {
403-
icon: 'icon-deck',
404-
iconTitle: t('files_sharing', 'Deck board'),
405-
}
406-
case ShareType.Sciencemesh:
407-
return {
408-
icon: 'icon-sciencemesh',
409-
iconTitle: t('files_sharing', 'ScienceMesh'),
410-
}
411-
default:
412-
return {}
413-
}
414-
},
415-
416-
/**
417-
* Format shares for the multiselect options
418-
*
419-
* @param {object} result select entry item
420-
* @return {object}
421-
*/
422-
formatForMultiselect(result) {
423-
let subname
424-
if (result.value.shareType === ShareType.User && this.config.shouldAlwaysShowUnique) {
425-
subname = result.shareWithDisplayNameUnique ?? ''
426-
} else if ((result.value.shareType === ShareType.Remote
427-
|| result.value.shareType === ShareType.RemoteGroup
428-
) && result.value.server) {
429-
subname = t('files_sharing', 'on {server}', { server: result.value.server })
430-
} else if (result.value.shareType === ShareType.Email) {
431-
subname = result.value.shareWith
432-
} else {
433-
subname = result.shareWithDescription ?? ''
434-
}
435-
436-
return {
437-
shareWith: result.value.shareWith,
438-
shareType: result.value.shareType,
439-
user: result.uuid || result.value.shareWith,
440-
isNoUser: result.value.shareType !== ShareType.User,
441-
displayName: result.name || result.label,
442-
subname,
443-
shareWithDisplayNameUnique: result.shareWithDisplayNameUnique || '',
444-
...this.shareTypeToIcon(result.value.shareType),
445-
}
446190
},
447191
},
448192
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { getCurrentUser } from '@nextcloud/auth'
2+
import { ShareType } from '@nextcloud/sharing'
3+
4+
/**
5+
* Filter out existing shares from the provided shares search results
6+
*
7+
* @param {object[]} shares the array of shares object
8+
* @param {object} context the Vue object
9+
* @param {object[]} context.reshare reshare from SharingInput Vue instance
10+
* @param {object[]} context.shares shares from SharingInput Vue instance
11+
* @param {object[]} context.linkShares linkShares from SharingInput Vue instance
12+
* @return {object[]}
13+
*/
14+
export default (shares, context) => {
15+
return shares.reduce((arr, share) => {
16+
// only check proper objects
17+
if (typeof share !== 'object') {
18+
return arr
19+
}
20+
try {
21+
if (share.value.shareType === ShareType.User) {
22+
// filter out current user
23+
if (share.value.shareWith === getCurrentUser().uid) {
24+
return arr
25+
}
26+
27+
// filter out the owner of the share
28+
if (context.reshare && share.value.shareWith === context.reshare.owner) {
29+
return arr
30+
}
31+
}
32+
33+
// filter out existing mail shares
34+
if (share.value.shareType === ShareType.Email) {
35+
const emails = context.linkShares.map(elem => elem.shareWith)
36+
if (emails.indexOf(share.value.shareWith.trim()) !== -1) {
37+
return arr
38+
}
39+
} else { // filter out existing shares
40+
// creating an object of uid => type
41+
const sharesObj = context.shares.reduce((obj, elem) => {
42+
obj[elem.shareWith] = elem.type
43+
return obj
44+
}, {})
45+
46+
// if shareWith is the same and the share type too, ignore it
47+
const key = share.value.shareWith.trim()
48+
if (key in sharesObj
49+
&& sharesObj[key] === share.value.shareType) {
50+
return arr
51+
}
52+
}
53+
54+
// ALL GOOD
55+
// let's add the suggestion
56+
arr.push(share)
57+
} catch {
58+
return arr
59+
}
60+
return arr
61+
}, [])
62+
};

0 commit comments

Comments
 (0)