Skip to content

Commit 52670e7

Browse files
cubapthehabes
andauthored
478 role action false success (#506)
* Update project.test.js * Validate API responses and improve error handling Add a private #validateResponse method to parse JSON/text responses, extract meaningful error messages, and detect semantic failures even when HTTP status is 200. Replace ad-hoc response.ok checks across member/role API calls (removeMember, promoteUser, removeLeaderRole, revokeWriteAccess, setUserRoles, updateRoles) to use #validateResponse, return the parsed payload when available, surface user-friendly messages via userMessage/alerts, and rethrow errors so callers can handle them. This change prevents silent success on semantically failed responses and centralizes response validation/decoding. * Changes during review * Changes during review * Ready to merge, but needs fixed because the manage collaborators interface is having all kinds of trouble --------- Co-authored-by: Bryan Haberberger <bryan.j.haberberger@slu.edu>
1 parent 2926be8 commit 52670e7

File tree

2 files changed

+292
-220
lines changed

2 files changed

+292
-220
lines changed

api/Project.js

Lines changed: 125 additions & 202 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,47 @@ import { userMessage } from "../components/iiif-tools/index.js"
2323

2424
export default class Project {
2525

26-
#authentication
2726
#isLoaded
2827

28+
// We eventually want this caught and managed better upstream, but we might get 200 responses here that do not represent success (#478).
29+
async #validateResponse(response, fallbackMessage) {
30+
const contentType = response?.headers?.get?.('content-type') ?? ''
31+
let payload = null
32+
33+
if (contentType.includes('application/json')) {
34+
payload = await response.json().catch(() => null)
35+
} else {
36+
const text = await response.text().catch(() => '')
37+
if (text) {
38+
try {
39+
payload = JSON.parse(text)
40+
} catch {
41+
payload = { message: text }
42+
}
43+
}
44+
}
45+
46+
if (!response.ok) {
47+
const errorMessage = payload?.message ?? payload?.error ?? payload?.errorResponse?.errmsg ?? `${fallbackMessage}: ${response.status}`
48+
throw new Error(errorMessage)
49+
}
50+
51+
const hasSemanticError = Boolean(
52+
payload?.error ||
53+
payload?.errorResponse ||
54+
payload?.ok === false ||
55+
payload?.success === false ||
56+
(typeof payload?.status === 'number' && payload.status >= 400)
57+
)
58+
59+
if (hasSemanticError) {
60+
const semanticMessage = payload?.message ?? payload?.error?.message ?? payload?.errorResponse?.errmsg ?? payload?.error ?? fallbackMessage
61+
throw new Error(semanticMessage)
62+
}
63+
64+
return payload
65+
}
66+
2967
constructor(_id) {
3068
if (typeof _id !== "string") {
3169
throw new Error("Project ID must be a string")
@@ -103,163 +141,119 @@ export default class Project {
103141
}
104142

105143
async addMember(email, roles = undefined) {
106-
try {
107-
const AUTH_TOKEN = TPEN.getAuthorization() ?? TPEN.login()
108-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/invite-member`, {
109-
headers: {
110-
Authorization: `Bearer ${AUTH_TOKEN}`,
111-
'Content-Type': 'application/json',
112-
},
113-
method: "POST",
114-
body: JSON.stringify({ email, roles }),
115-
})
116-
if (!response.ok) {
117-
const errorData = await response.json()
118-
throw new Error(errorData.message || `Failed to invite collaborator: ${response.statusText}`)
119-
}
120-
121-
return await response.json()
122-
} catch (error) {
123-
userMessage(error.message)
124-
}
144+
const AUTH_TOKEN = TPEN.getAuthorization() ?? TPEN.login()
145+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/invite-member`, {
146+
headers: {
147+
Authorization: `Bearer ${AUTH_TOKEN}`,
148+
'Content-Type': 'application/json',
149+
},
150+
method: "POST",
151+
body: JSON.stringify({ email, roles }),
152+
})
153+
const payload = await this.#validateResponse(response, 'Failed to invite collaborator')
154+
return payload ?? response
125155
}
126156

127157
async removeMember(userId) {
128-
try {
129-
const token = TPEN.getAuthorization() ?? TPEN.login()
130-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/remove-member`, {
131-
method: "POST",
132-
headers: {
133-
Authorization: `Bearer ${token}`,
134-
'Content-Type': 'application/json',
135-
},
136-
body: JSON.stringify({ userId }),
137-
})
138-
if (!response.ok) {
139-
throw new Error(`Error removing member: ${response.status}`)
140-
}
158+
const token = TPEN.getAuthorization() ?? TPEN.login()
159+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/remove-member`, {
160+
method: "POST",
161+
headers: {
162+
Authorization: `Bearer ${token}`,
163+
'Content-Type': 'application/json',
164+
},
165+
body: JSON.stringify({ userId }),
166+
})
167+
const payload = await this.#validateResponse(response, 'Error removing member')
141168

142-
delete this.collaborators[userId]
143-
return await response
144-
} catch (error) {
145-
userMessage(error.message)
146-
}
169+
delete this.collaborators[userId]
170+
return payload ?? response
147171
}
148172

149173
async makeLeader(userId) {
150-
try {
151-
const token = TPEN.getAuthorization() ?? TPEN.login()
152-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/addRoles`, {
153-
method: "POST",
154-
headers: {
155-
Authorization: `Bearer ${token}`,
156-
'Content-Type': 'application/json',
157-
},
158-
body: JSON.stringify(["LEADER"]),
159-
})
160-
if (!response.ok) {
161-
throw new Error(`Error promoting user to LEADER: ${response.status}`)
162-
}
174+
const token = TPEN.getAuthorization() ?? TPEN.login()
175+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/addRoles`, {
176+
method: "POST",
177+
headers: {
178+
Authorization: `Bearer ${token}`,
179+
'Content-Type': 'application/json',
180+
},
181+
body: JSON.stringify(["LEADER"]),
182+
})
183+
const payload = await this.#validateResponse(response, 'Error promoting user to LEADER')
163184

164-
if (this.collaborators[userId] && !this.collaborators[userId].roles.includes("LEADER")) {
165-
this.collaborators[userId].roles.push("LEADER")
166-
}
167-
return response
168-
} catch (error) {
169-
userMessage(error.message)
185+
if (this.collaborators[userId] && !this.collaborators[userId].roles.includes("LEADER")) {
186+
this.collaborators[userId].roles.push("LEADER")
170187
}
188+
return payload ?? response
171189
}
172190

173191
async demoteLeader(userId) {
174-
try {
175-
const token = TPEN.getAuthorization() ?? TPEN.login()
176-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/removeRoles`, {
177-
method: "POST",
178-
headers: {
179-
Authorization: `Bearer ${token}`,
180-
'Content-Type': 'application/json',
181-
},
182-
body: JSON.stringify(["LEADER"]),
183-
})
184-
if (!response.ok) {
185-
throw new Error(`Error removing LEADER role: ${response.status}`)
186-
}
192+
const token = TPEN.getAuthorization() ?? TPEN.login()
193+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/removeRoles`, {
194+
method: "POST",
195+
headers: {
196+
Authorization: `Bearer ${token}`,
197+
'Content-Type': 'application/json',
198+
},
199+
body: JSON.stringify(["LEADER"]),
200+
})
201+
const payload = await this.#validateResponse(response, 'Error removing LEADER role')
187202

188-
if (this.collaborators[userId]) {
189-
this.collaborators[userId].roles = this.collaborators[userId].roles.filter(role => role !== "LEADER")
190-
}
191-
return response
192-
} catch (error) {
193-
userMessage(error.message)
203+
if (this.collaborators[userId]) {
204+
this.collaborators[userId].roles = this.collaborators[userId].roles.filter(role => role !== "LEADER")
194205
}
206+
return payload ?? response
195207
}
196208

197209
async setToViewer(userId) {
198-
try {
199-
const token = TPEN.getAuthorization() ?? TPEN.login()
200-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/setRoles`, {
201-
method: "PUT",
202-
headers: {
203-
Authorization: `Bearer ${token}`,
204-
'Content-Type': 'application/json',
205-
},
206-
body: JSON.stringify(["VIEWER"]),
207-
})
208-
if (!response.ok) {
209-
throw new Error(`Error revoking write access: ${response.status}`)
210-
}
211-
if (this.collaborators[userId]) {
212-
this.collaborators[userId].roles = ["VIEWER"]
213-
} return response
214-
} catch (error) {
215-
userMessage(error.message)
210+
const token = TPEN.getAuthorization() ?? TPEN.login()
211+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/setRoles`, {
212+
method: "PUT",
213+
headers: {
214+
Authorization: `Bearer ${token}`,
215+
'Content-Type': 'application/json',
216+
},
217+
body: JSON.stringify(["VIEWER"]),
218+
})
219+
const payload = await this.#validateResponse(response, 'Error revoking write access')
220+
if (this.collaborators[userId]) {
221+
this.collaborators[userId].roles = ["VIEWER"]
216222
}
223+
return payload ?? response
217224
}
218225

219226
async cherryPickRoles(userId, roles) {
220-
try {
221-
const token = TPEN.getAuthorization() ?? TPEN.login()
222-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/setRoles`, {
223-
method: "PUT",
224-
headers: {
225-
Authorization: `Bearer ${token}`,
226-
'Content-Type': 'application/json',
227-
},
228-
body: JSON.stringify({ roles }),
229-
})
230-
if (!response.ok) {
231-
throw new Error(`Error setting user roles: ${response.status}`)
232-
}
227+
const token = TPEN.getAuthorization() ?? TPEN.login()
228+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userId}/setRoles`, {
229+
method: "PUT",
230+
headers: {
231+
Authorization: `Bearer ${token}`,
232+
'Content-Type': 'application/json',
233+
},
234+
body: JSON.stringify({ roles }),
235+
})
236+
const payload = await this.#validateResponse(response, 'Error setting user roles')
233237

234-
if (this.collaborators[userId]) {
235-
this.collaborators[userId].roles = roles
236-
}
237-
return response
238-
} catch (error) {
239-
userMessage(error.message)
238+
if (this.collaborators[userId]) {
239+
this.collaborators[userId].roles = roles
240240
}
241+
return payload ?? response
241242
}
242243

243244
async transferOwnership(userId) {
244-
try {
245-
const token = TPEN.getAuthorization() ?? TPEN.login()
246-
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/switch/owner`, {
247-
method: "POST",
248-
headers: {
249-
Authorization: `Bearer ${token}`,
250-
'Content-Type': 'application/json',
251-
},
252-
body: JSON.stringify({ newOwnerId: userId })
253-
})
245+
const token = TPEN.getAuthorization() ?? TPEN.login()
246+
const response = await fetch(`${TPEN.servicesURL}/project/${this._id}/switch/owner`, {
247+
method: "POST",
248+
headers: {
249+
Authorization: `Bearer ${token}`,
250+
'Content-Type': 'application/json',
251+
},
252+
body: JSON.stringify({ newOwnerId: userId })
253+
})
254254

255-
if (!response.ok) {
256-
throw new Error("Failed to update roles")
257-
}
258-
return response
259-
} catch (error) {
260-
console.error("Error updating roles:", error)
261-
eventDispatcher.dispatch('tpen-alert', { message: "Failed to update roles. Please try again." })
262-
}
255+
const payload = await this.#validateResponse(response, 'Failed to update roles')
256+
return payload ?? response
263257
}
264258

265259
setMetadata(metadata) {
@@ -299,77 +293,6 @@ export default class Project {
299293
return this.save()
300294
}
301295

302-
async inviteCollaborator(email, roles) {
303-
return fetch(`${TPEN.servicesURL}/project/${this._id}/invite-member`, {
304-
method: "POST",
305-
headers: new Headers({
306-
Authorization: `Bearer ${this.#authentication}`,
307-
"Content-Type": "application/json"
308-
}),
309-
body: JSON.stringify({ email, roles })
310-
}).catch(err => Promise.reject(err))
311-
}
312-
313-
async removeCollaborator(userID) {
314-
// userID is the _id (Hex String) of the user to remove from the project
315-
if (!this.collaborators?.[userID]) {
316-
return Promise.reject(new Error("User not found in collaborators list"))
317-
}
318-
return fetch(`${TPEN.servicesURL}/project/${this._id}/remove-member`, {
319-
method: "POST",
320-
headers: new Headers({
321-
Authorization: `Bearer ${this.#authentication}`,
322-
"Content-Type": "application/json"
323-
}),
324-
body: JSON.stringify({ userID })
325-
}).catch(err => Promise.reject(err))
326-
}
327-
328-
async addCollaboratorRole(userID, roles) {
329-
// role is a string value of the role to add to the user
330-
if (!this.collaborators?.[userID]) {
331-
return Promise.reject(new Error("User not found in collaborators list"))
332-
}
333-
return fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userID}/addRoles`, {
334-
method: "POST",
335-
headers: new Headers({
336-
Authorization: `Bearer ${this.#authentication}`,
337-
"Content-Type": "application/json"
338-
}),
339-
body: JSON.stringify(roles)
340-
}).catch(err => Promise.reject(err))
341-
}
342-
343-
async removeCollaboratorRole(userID, roles) {
344-
// role is a string value of the role to remove from the user
345-
if (!this.collaborators?.[userID]) {
346-
return Promise.reject(new Error("User not found in collaborators list"))
347-
}
348-
return fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userID}/removeRoles`, {
349-
method: "POST",
350-
headers: new Headers({
351-
Authorization: `Bearer ${this.#authentication}`,
352-
"Content-Type": "application/json"
353-
}),
354-
body: JSON.stringify(roles)
355-
}).catch(err => Promise.reject(err))
356-
}
357-
358-
async setCollaboratorRoles(userID, roles) {
359-
// role is a string value of the role to set for the user
360-
if (!this.collaborators?.[userID]) {
361-
return Promise.reject(new Error("User not found in collaborators list"))
362-
}
363-
return fetch(`${TPEN.servicesURL}/project/${this._id}/collaborator/${userID}/setRoles`, {
364-
method: "PUT",
365-
headers: new Headers({
366-
Authorization: `Bearer ${this.#authentication}`,
367-
"Content-Type": "application/json"
368-
}),
369-
body: JSON.stringify(roles)
370-
}).catch(err => Promise.reject(err))
371-
}
372-
373296
async storeInterfacesCustomization(customizations, replace = false) {
374297
if (!this.#isLoaded) {
375298
throw new Error("Project must be loaded before storing interface customizations")

0 commit comments

Comments
 (0)