Skip to content

Commit 963443e

Browse files
NejcZdovcNejcZdovc
committed
Follow up for sites splits PR brave#10296
Resolves brave#10296 Resolves brave#12378 Auditors: Test Plan:
1 parent db4b3b9 commit 963443e

8 files changed

+1977
-137
lines changed

‎app/common/lib/historyUtil.js

+28-8
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,32 @@ const sortTimeDescending = (left, right) => {
1515
}
1616

1717
const getHistory = (sites) => {
18-
sites = makeImmutable(sites) ? makeImmutable(sites).toList() : new Immutable.List()
18+
if (sites == null) {
19+
return Immutable.List()
20+
}
21+
22+
sites = makeImmutable(sites) ? makeImmutable(sites).toList() : Immutable.List()
1923
return sites
2024
.sort(sortTimeDescending)
2125
.slice(0, aboutHistoryMaxEntries)
2226
}
2327

2428
const getDayString = (entry, locale) => {
29+
if (entry == null) {
30+
return ''
31+
}
32+
2533
const lastAccessedTime = entry.get('lastAccessedTime')
2634
return lastAccessedTime
2735
? new Date(lastAccessedTime).toLocaleDateString(locale, { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' })
2836
: ''
2937
}
3038

3139
const groupEntriesByDay = (history, locale) => {
40+
if (history == null) {
41+
return Immutable.List()
42+
}
43+
3244
const reduced = history.reduce((previousValue, currentValue, currentIndex) => {
3345
const result = currentIndex === 1 ? [] : previousValue
3446
if (currentIndex === 1) {
@@ -44,31 +56,38 @@ const groupEntriesByDay = (history, locale) => {
4456
}
4557
return result
4658
})
59+
4760
if (reduced) {
4861
return Immutable.fromJS(
4962
Array.isArray(reduced)
5063
? reduced
5164
: [{date: getDayString(reduced, locale), entries: [reduced]}]
5265
)
5366
}
54-
return Immutable.fromJS([])
67+
return Immutable.List()
5568
}
5669

5770
/**
5871
* Return an array with ALL entries.
5972
* Format is expected to be array containing one array per day.
6073
*/
6174
const totalEntries = (entriesByDay) => {
62-
entriesByDay = makeImmutable(entriesByDay) || new Immutable.List()
75+
if (entriesByDay == null) {
76+
return Immutable.List()
77+
}
78+
79+
entriesByDay = makeImmutable(entriesByDay)
6380

64-
let result = new Immutable.List()
65-
entriesByDay.forEach((entry) => {
66-
result = result.push(entry.get('entries'))
81+
return entriesByDay.map((entry) => {
82+
return entry.get('entries')
6783
})
68-
return result
6984
}
7085

7186
const prepareHistoryEntry = (siteDetail) => {
87+
if (siteDetail == null) {
88+
return Immutable.Map()
89+
}
90+
7291
const time = siteDetail.has('lastAccessedTime')
7392
? siteDetail.get('lastAccessedTime')
7493
: new Date().getTime()
@@ -82,7 +101,7 @@ const prepareHistoryEntry = (siteDetail) => {
82101
count: 1,
83102
themeColor: siteDetail.get('themeColor'),
84103
favicon: siteDetail.get('favicon', siteDetail.get('icon')),
85-
key: getKey(siteDetail),
104+
key: module.exports.getKey(siteDetail),
86105
skipSync: siteDetail.get('skipSync', null)
87106
})
88107
}
@@ -142,6 +161,7 @@ const getKey = (siteDetail) => {
142161
return location + '|' +
143162
(siteDetail.get('partitionNumber') || 0)
144163
}
164+
145165
return null
146166
}
147167

‎app/common/state/bookmarkFoldersState.js

+50-15
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,39 @@ const bookmarkFoldersState = {
3636

3737
getFolder: (state, folderKey) => {
3838
state = validateState(state)
39+
40+
if (folderKey == null) {
41+
return Immutable.Map()
42+
}
43+
3944
folderKey = folderKey.toString()
4045
return state.getIn([STATE_SITES.BOOKMARK_FOLDERS, folderKey], Immutable.Map())
4146
},
4247

4348
getFoldersByParentId: (state, parentFolderId) => {
4449
state = validateState(state)
4550

51+
if (parentFolderId == null) {
52+
return Immutable.List()
53+
}
54+
4655
const folders = bookmarkOrderCache.getFoldersByParentId(state, parentFolderId)
4756
return folders.map(folder => bookmarkFoldersState.getFolder(state, folder.get('key')))
4857
},
4958

5059
addFolder: (state, folderDetails, destinationKey) => {
5160
state = validateState(state)
5261

62+
if (folderDetails == null) {
63+
return state
64+
}
65+
66+
folderDetails = makeImmutable(folderDetails)
67+
68+
if (folderDetails.get('key') == null) {
69+
return state
70+
}
71+
5372
state = state.setIn([STATE_SITES.BOOKMARK_FOLDERS, folderDetails.get('key')], folderDetails)
5473
state = bookmarkOrderCache.addFolderToCache(state, folderDetails.get('parentFolderId'), folderDetails.get('key'), destinationKey)
5574
return state
@@ -78,24 +97,23 @@ const bookmarkFoldersState = {
7897
},
7998

8099
removeFolder: (state, folderKey) => {
81-
const bookmarksState = require('./bookmarksState')
82-
const folders = bookmarkFoldersState.getFolders(state)
100+
state = validateState(state)
83101
const folder = bookmarkFoldersState.getFolder(state, folderKey)
84102

85103
if (folder.isEmpty()) {
86104
return state
87105
}
88106

107+
const bookmarksState = require('./bookmarksState')
108+
const folders = bookmarkFoldersState.getFolders(state)
109+
89110
if (getSetting(settings.SYNC_ENABLED) === true) {
90111
syncActions.removeSites([folder.toJS()])
91112
}
92113

93114
folders.filter(folder => folder.get('parentFolderId') === Number(folderKey))
94115
.map(folder => {
95-
state = bookmarksState.removeBookmarksByParentId(state, folder.get('folderId'))
96116
state = bookmarkFoldersState.removeFolder(state, folder.get('folderId'))
97-
state = bookmarkOrderCache.removeCacheParent(state, folder.get('folderId'))
98-
state = bookmarkOrderCache.removeCacheKey(state, folder.get('parentFolderId'), folderKey)
99117
})
100118

101119
state = bookmarksState.removeBookmarksByParentId(state, folderKey)
@@ -105,14 +123,15 @@ const bookmarkFoldersState = {
105123
},
106124

107125
/**
108-
* Get all folders except provided folder
126+
* Get a list of all folders except provided folder
109127
* @param state
110-
* @param folderKey
128+
* @param {number} folderKey
111129
* @param parentFolderId
112130
* @param labelPrefix
113-
* @returns {Array}
131+
* @returns {Array} - each entry with folder id and label (title)
114132
*/
115133
getFoldersWithoutKey: (state, folderKey, parentFolderId = 0, labelPrefix = '') => {
134+
state = validateState(state)
116135
let folders = []
117136
const results = bookmarkFoldersState.getFoldersByParentId(state, parentFolderId)
118137

@@ -128,27 +147,36 @@ const bookmarkFoldersState = {
128147
folderId: folder.get('folderId'),
129148
label
130149
})
131-
const subSites = bookmarkFoldersState.getFoldersWithoutKey(state, folderKey, folder.get('folderId'), (label || '') + ' / ')
132-
folders = folders.concat(subSites)
150+
const subFolders = bookmarkFoldersState.getFoldersWithoutKey(state, folderKey, folder.get('folderId'), (label || '') + ' / ')
151+
folders = folders.concat(subFolders)
133152
}
134153

135154
return folders
136155
},
137156

138157
moveFolder: (state, folderKey, destinationKey, append, moveIntoParent) => {
139-
const bookmarksState = require('./bookmarksState')
158+
state = validateState(state)
140159
let folder = bookmarkFoldersState.getFolder(state, folderKey)
141-
let destinationItem = bookmarksState.findBookmark(state, destinationKey)
142-
143160
if (folder.isEmpty()) {
144161
return state
145162
}
146163

164+
const bookmarksState = require('./bookmarksState')
165+
let destinationItem = bookmarksState.findBookmark(state, destinationKey)
166+
const numKey = Number(destinationKey)
167+
if (destinationItem.isEmpty() && numKey !== -1 && numKey !== 0) {
168+
return state
169+
}
170+
147171
if (moveIntoParent || destinationItem.get('parentFolderId') !== folder.get('parentFolderId')) {
148-
const parentFolderId = destinationItem.get('type') === siteTags.BOOKMARK
172+
let parentFolderId = destinationItem.get('type') === siteTags.BOOKMARK
149173
? destinationItem.get('parentFolderId')
150174
: destinationItem.get('folderId')
151175

176+
if (parentFolderId == null) {
177+
parentFolderId = destinationKey
178+
}
179+
152180
state = bookmarkOrderCache.removeCacheKey(state, folder.get('parentFolderId'), folderKey)
153181
folder = folder.set('parentFolderId', Number(parentFolderId))
154182
const newKey = bookmarkFoldersUtil.getKey(folder)
@@ -169,7 +197,14 @@ const bookmarkFoldersState = {
169197
},
170198

171199
setWidth: (state, key, width) => {
172-
return state.setIn([STATE_SITES.BOOKMARK_FOLDERS, key, 'width'], parseFloat(width))
200+
state = validateState(state)
201+
width = parseFloat(width)
202+
203+
if (key == null || isNaN(width)) {
204+
return state
205+
}
206+
207+
return state.setIn([STATE_SITES.BOOKMARK_FOLDERS, key, 'width'], width)
173208
}
174209
}
175210

‎app/common/state/bookmarksState.js

+49-6
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,16 @@ const validateState = function (state) {
3333
const bookmarksState = {
3434
getBookmarks: (state) => {
3535
state = validateState(state)
36-
return state.get(STATE_SITES.BOOKMARKS)
36+
return state.get(STATE_SITES.BOOKMARKS) || Immutable.Map()
3737
},
3838

3939
getBookmark: (state, key) => {
4040
state = validateState(state)
41+
42+
if (key == null) {
43+
return Immutable.Map()
44+
}
45+
4146
return state.getIn([STATE_SITES.BOOKMARKS, key], Immutable.Map())
4247
},
4348

@@ -48,6 +53,11 @@ const bookmarksState = {
4853
*/
4954
findBookmark: (state, key) => {
5055
state = validateState(state)
56+
57+
if (key == null) {
58+
return Immutable.Map()
59+
}
60+
5161
let bookmark = bookmarksState.getBookmark(state, key)
5262
if (bookmark.isEmpty()) {
5363
bookmark = bookmarkFoldersState.getFolder(state, key)
@@ -75,8 +85,15 @@ const bookmarksState = {
7585

7686
addBookmark: (state, bookmarkDetail, destinationKey, isLeftSide) => {
7787
state = validateState(state)
88+
89+
if (bookmarkDetail == null) {
90+
return state
91+
}
92+
93+
bookmarkDetail = makeImmutable(bookmarkDetail)
94+
7895
const key = bookmarkDetail.get('key')
79-
if (key === null) {
96+
if (key == null) {
8097
return state
8198
}
8299

@@ -92,6 +109,13 @@ const bookmarksState = {
92109
editBookmark: (state, oldBookmark, bookmarkDetail) => {
93110
state = validateState(state)
94111

112+
if (oldBookmark == null || bookmarkDetail == null) {
113+
return state
114+
}
115+
116+
bookmarkDetail = makeImmutable(bookmarkDetail)
117+
oldBookmark = makeImmutable(oldBookmark)
118+
95119
const newKey = bookmarkDetail.get('key')
96120
const editKey = oldBookmark.get('key')
97121

@@ -111,6 +135,10 @@ const bookmarksState = {
111135
removeBookmark: (state, bookmarkKey) => {
112136
state = validateState(state)
113137

138+
if (bookmarkKey == null) {
139+
return state
140+
}
141+
114142
const bookmark = bookmarksState.getBookmark(state, bookmarkKey)
115143

116144
if (bookmark.isEmpty()) {
@@ -140,24 +168,29 @@ const bookmarksState = {
140168
return state
141169
}
142170

171+
parentFolderId = Number(parentFolderId)
172+
143173
const syncEnabled = getSetting(settings.SYNC_ENABLED) === true
144174
const removedBookmarks = []
145175
const bookmarks = bookmarksState.getBookmarks(state)
146176
.filter(bookmark => {
147-
if (bookmark.get('parentFolderId') !== Number(parentFolderId)) {
177+
if (bookmark.get('parentFolderId') !== parentFolderId) {
148178
return true
149179
}
180+
150181
if (syncEnabled) {
151182
removedBookmarks.push(bookmark.toJS())
152183
}
153184

154185
state = bookmarkLocationCache.removeCacheKey(state, bookmark.get('location'), bookmark.get('key'))
186+
state = bookmarkOrderCache.removeCacheKey(state, bookmark.get('parentFolderId'), bookmark.get('key'))
155187
return false
156188
})
157189

158190
if (syncEnabled && removedBookmarks.length) {
159191
syncActions.removeSites(removedBookmarks)
160192
}
193+
161194
return state.set(STATE_SITES.BOOKMARKS, bookmarks)
162195
},
163196

@@ -195,12 +228,16 @@ const bookmarksState = {
195228

196229
const bookmarkUtil = require('../lib/bookmarkUtil')
197230
let bookmark = bookmarksState.getBookmark(state, bookmarkKey)
198-
let destinationItem = bookmarksState.findBookmark(state, destinationKey)
199-
200231
if (bookmark.isEmpty()) {
201232
return state
202233
}
203234

235+
let destinationItem = bookmarksState.findBookmark(state, destinationKey)
236+
const numKey = Number(destinationKey)
237+
if (destinationItem.isEmpty() && numKey !== -1 && numKey !== 0) {
238+
return state
239+
}
240+
204241
// move bookmark into a new folder
205242
if (moveIntoParent || destinationItem.get('parentFolderId') !== bookmark.get('parentFolderId')) {
206243
let parentFolderId = destinationItem.get('type') === siteTags.BOOKMARK
@@ -251,7 +288,13 @@ const bookmarksState = {
251288
},
252289

253290
setWidth: (state, key, width) => {
254-
return state.setIn([STATE_SITES.BOOKMARKS, key, 'width'], parseFloat(width))
291+
width = parseFloat(width)
292+
293+
if (key == null || isNaN(width)) {
294+
return state
295+
}
296+
297+
return state.setIn([STATE_SITES.BOOKMARKS, key, 'width'], width)
255298
}
256299
}
257300

0 commit comments

Comments
 (0)