Skip to content

Commit 6fe9d7d

Browse files
jaw6maestromac
andauthored
Allow org admins to add members as co-authors (#20008)
* Basics might be working? * Stop propagating button clicks in autocomplete pills * Better blank slate * Better location to stop propagation * Remove author_id from org co-authors * Move UserStore and try testing it * Remove extraneous comments * OH... that's what that does! * Very basic testing * Re-organize javascripts * Rename & re-org for testing * Cleanup * More tests * Remove unnecessary nesting * Coninuing to try to bump coverage * Include /packs/ in code coverage metric * Try tweaking jest coverage more? We probably can't collect coverage from all of packs/* (because coverage is too low) but maybe we can try to opt-in for newer areas as we go? * Relocate JS tests, for build & coverage * User ID exception on search, not fetch * Remove commented-out console.log --------- Co-authored-by: Mac Siri <[email protected]>
1 parent 612c402 commit 6fe9d7d

20 files changed

+552
-4
lines changed

app/controllers/articles_controller.rb

+2-1
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,9 @@ def article_params_json
328328

329329
# NOTE: the organization logic is still a little counter intuitive but this should
330330
# fix the bug <https://github.com/forem/forem/issues/2871>
331-
if params["article"]["user_id"] && org_admin_user_change_privilege
331+
if org_admin_user_change_privilege
332332
allowed_params << :user_id
333+
allowed_params << :co_author_ids_list
333334
elsif params["article"]["organization_id"] && allowed_to_change_org_id?
334335
# change the organization of the article only if explicitly asked to do so
335336
allowed_params << :organization_id

app/controllers/organizations_controller.rb

+5
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ def generate_new_secret
9797
def members
9898
@organization = Organization.find_by(slug: params[:slug])
9999
@members = @organization.users
100+
101+
respond_to do |format|
102+
format.json { render json: @members.to_json(only: %i[id name username]) }
103+
format.html
104+
end
100105
end
101106

102107
private
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`convertCoauthorIdsToUsernameInputs when there *is* a pre-existing id value renders matching snapshot 1`] = `
4+
"
5+
<div class=\\"crayons-field mb-4\\">
6+
<label for=\\"user_id\\">Author</label>
7+
<select class=\\"crayons-select\\" name=\\"article[user_id]\\" id=\\"article_user_id\\">
8+
<option selected=\\"selected\\" value=\\"1\\">
9+
Alice
10+
</option>
11+
<option value=\\"2\\">
12+
Bob
13+
</option>
14+
<option value=\\"3\\">
15+
Charlie
16+
</option>
17+
</select>
18+
</div>
19+
20+
<div class=\\"crayons-field mb-4\\">
21+
<label for=\\"co_author_ids\\">Co-authors</label>
22+
<div class=\\"crayons-field\\"><span aria-hidden=\\"true\\" class=\\"absolute pointer-events-none opacity-0 p-2\\"></span><label id=\\"multi-select-label\\" class=\\"screen-reader-only\\">Add up to 4</label><span id=\\"input-description\\" class=\\"screen-reader-only\\">Maximum 4 selections</span><div class=\\"screen-reader-only\\"><p>Selected items:</p><ul aria-live=\\"assertive\\" aria-atomic=\\"false\\" aria-relevant=\\"additions removals\\" class=\\"screen-reader-only list-none\\"></ul></div><div class=\\"c-autocomplete--multi relative\\"><div role=\\"combobox\\" aria-haspopup=\\"listbox\\" aria-expanded=\\"false\\" aria-owns=\\"listbox1\\" class=\\"c-autocomplete--multi__wrapper-border crayons-textfield flex items-center cursor-text\\"><ul id=\\"combo-selected\\" class=\\"list-none flex flex-wrap w-100\\"><li style=\\"order: 1;\\" class=\\"c-autocomplete--multi__selection-list-item w-max\\"><div role=\\"group\\" aria-label=\\"Charlie\\" class=\\"flex mr-1 mb-1 w-max\\"><button type=\\"button\\" aria-label=\\"Edit Charlie\\" class=\\"c-btn c-btn--secondary c-autocomplete--multi__selected p-1 cursor-text\\">Charlie</button><button type=\\"button\\" aria-label=\\"Remove Charlie\\" class=\\"c-btn c-btn--secondary c-autocomplete--multi__selected p-1\\"><svg width=\\"24\\" height=\\"24\\" viewBox=\\"0 0 24 24\\" xmlns=\\"http://www.w3.org/2000/svg\\" class=\\"crayons-icon\\"><path d=\\"m12 10.586 4.95-4.95 1.414 1.414-4.95 4.95 4.95 4.95-1.414 1.414-4.95-4.95-4.95 4.95-1.414-1.414 4.95-4.95-4.95-4.95L7.05 5.636l4.95 4.95z\\"></path></svg></button></div></li><li style=\\"order: 2;\\" class=\\"self-center\\"><input id=\\"autoarticle_ID_co_author_ids_list\\" autocomplete=\\"off\\" aria-autocomplete=\\"list\\" aria-labelledby=\\"multi-select-label selected-items-list\\" aria-describedby=\\"input-description\\" aria-disabled=\\"false\\" type=\\"text\\" placeholder=\\"Add another...\\" class=\\"c-autocomplete--multi__input\\"></li></ul></div></div>
23+
<input id=\\"article_ID_co_author_ids_list\\" class=\\"article_org_co_author_ids_list\\" name=\\"article[co_author_ids_list]\\" data-fetch-users=\\"/org7053/members.json\\" type=\\"hidden\\" value=\\"3\\">
24+
</div>
25+
</div>
26+
27+
<button type=\\"submit\\" class=\\"crayons-btn\\">Save</button>
28+
"
29+
`;
30+
31+
exports[`convertCoauthorIdsToUsernameInputs when there is no pre-existing id value renders matching snapshot 1`] = `
32+
"
33+
<div class=\\"crayons-field mb-4\\">
34+
<label for=\\"user_id\\">Author</label>
35+
<select class=\\"crayons-select\\" name=\\"article[user_id]\\" id=\\"article_user_id\\">
36+
<option selected=\\"selected\\" value=\\"1\\">
37+
Alice
38+
</option>
39+
<option value=\\"2\\">
40+
Bob
41+
</option>
42+
<option value=\\"3\\">
43+
Charlie
44+
</option>
45+
</select>
46+
</div>
47+
48+
<div class=\\"crayons-field mb-4\\">
49+
<label for=\\"co_author_ids\\">Co-authors</label>
50+
<div class=\\"crayons-field\\"><span aria-hidden=\\"true\\" class=\\"absolute pointer-events-none opacity-0 p-2\\"></span><label id=\\"multi-select-label\\" class=\\"screen-reader-only\\">Add up to 4</label><span id=\\"input-description\\" class=\\"screen-reader-only\\">Maximum 4 selections</span><div class=\\"screen-reader-only\\"><p>Selected items:</p><ul aria-live=\\"assertive\\" aria-atomic=\\"false\\" aria-relevant=\\"additions removals\\" class=\\"screen-reader-only list-none\\"></ul></div><div class=\\"c-autocomplete--multi relative\\"><div role=\\"combobox\\" aria-haspopup=\\"listbox\\" aria-expanded=\\"false\\" aria-owns=\\"listbox1\\" class=\\"c-autocomplete--multi__wrapper-border crayons-textfield flex items-center cursor-text\\"><ul id=\\"combo-selected\\" class=\\"list-none flex flex-wrap w-100\\"><li style=\\"order: 1;\\" class=\\"self-center\\"><input id=\\"autoarticle_ID_co_author_ids_list\\" autocomplete=\\"off\\" aria-autocomplete=\\"list\\" aria-labelledby=\\"multi-select-label selected-items-list\\" aria-describedby=\\"input-description\\" aria-disabled=\\"false\\" type=\\"text\\" placeholder=\\"Add up to 4...\\" class=\\"c-autocomplete--multi__input\\"></li></ul></div></div>
51+
<input id=\\"article_ID_co_author_ids_list\\" class=\\"article_org_co_author_ids_list\\" name=\\"article[co_author_ids_list]\\" data-fetch-users=\\"/org7053/members.json\\" type=\\"hidden\\" value=\\"\\">
52+
</div>
53+
</div>
54+
55+
<button type=\\"submit\\" class=\\"crayons-btn\\">Save</button>
56+
"
57+
`;

app/javascript/packs/__tests__/billboard.test.js renamed to app/javascript/__tests__/billboard.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getBillboard } from '../billboard';
1+
import { getBillboard } from '../packs/billboard';
22

33
describe('getBillboard', () => {
44
beforeEach(() => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import fetch from 'jest-fetch-mock';
2+
import '@testing-library/jest-dom';
3+
import userEvent from '@testing-library/user-event';
4+
5+
import { convertCoauthorIdsToUsernameInputs } from '../packs/dashboards/convertCoauthorIdsToUsernameInputs';
6+
7+
global.fetch = fetch;
8+
9+
function fakeFetchResponseJSON() {
10+
return JSON.stringify([
11+
{ name: 'Alice', username: 'alice', id: 1 },
12+
{ name: 'Bob', username: 'bob', id: 2 },
13+
{ name: 'Charlie', username: 'charlie', id: 3 },
14+
]);
15+
}
16+
17+
describe('convertCoauthorIdsToUsernameInputs', () => {
18+
beforeEach(() => {
19+
global.Honeybadger = { notify: jest.fn() };
20+
21+
fetch.resetMocks();
22+
fetch.mockResponse(fakeFetchResponseJSON());
23+
});
24+
25+
describe('when there is no pre-existing id value', () => {
26+
beforeEach(() => {
27+
window.document.body.innerHTML = `
28+
<form method="post">
29+
<div class="crayons-field mb-4">
30+
<label for="user_id">Author</label>
31+
<select class="crayons-select" name="article[user_id]" id="article_user_id">
32+
<option selected="selected" value="1">
33+
Alice
34+
</option>
35+
<option value="2">
36+
Bob
37+
</option>
38+
<option value="3">
39+
Charlie
40+
</option>
41+
</select>
42+
</div>
43+
44+
<div class="crayons-field mb-4">
45+
<label for="co_author_ids">Co-authors</label>
46+
<div class="crayons-field">
47+
<input id="article_ID_co_author_ids_list"
48+
class="article_org_co_author_ids_list"
49+
name="article[co_author_ids_list]"
50+
data-fetch-users="/org7053/members.json"
51+
type="text"
52+
value="" >
53+
</div>
54+
</div>
55+
56+
<button type="submit" class="crayons-btn">Save</button>
57+
</form>
58+
`;
59+
});
60+
61+
it('calls fetch, with exception for author ID', async () => {
62+
await convertCoauthorIdsToUsernameInputs();
63+
expect(fetch).toHaveBeenCalledWith('/org7053/members.json');
64+
});
65+
66+
it('makes the co-author field hidden', async () => {
67+
await convertCoauthorIdsToUsernameInputs();
68+
const co_author_field = document.getElementById(
69+
'article_ID_co_author_ids_list',
70+
);
71+
expect(co_author_field.type).toBe('hidden');
72+
});
73+
74+
it('renders matching snapshot', async () => {
75+
await convertCoauthorIdsToUsernameInputs();
76+
expect(document.forms[0].innerHTML).toMatchSnapshot();
77+
});
78+
79+
it('works as expected', async () => {
80+
await convertCoauthorIdsToUsernameInputs();
81+
const input = document.querySelector(
82+
"input[placeholder='Add up to 4...']",
83+
);
84+
input.focus();
85+
await userEvent.type(input, 'Bob,');
86+
87+
const hiddenField = document.querySelector(
88+
"input[name='article[co_author_ids_list]']",
89+
);
90+
expect(hiddenField.value).toBe('2');
91+
});
92+
});
93+
94+
describe('when there *is* a pre-existing id value', () => {
95+
beforeEach(() => {
96+
window.document.body.innerHTML = `
97+
<form method="post">
98+
<div class="crayons-field mb-4">
99+
<label for="user_id">Author</label>
100+
<select class="crayons-select" name="article[user_id]" id="article_user_id">
101+
<option selected="selected" value="1">
102+
Alice
103+
</option>
104+
<option value="2">
105+
Bob
106+
</option>
107+
<option value="3">
108+
Charlie
109+
</option>
110+
</select>
111+
</div>
112+
113+
<div class="crayons-field mb-4">
114+
<label for="co_author_ids">Co-authors</label>
115+
<div class="crayons-field">
116+
<input id="article_ID_co_author_ids_list"
117+
class="article_org_co_author_ids_list"
118+
name="article[co_author_ids_list]"
119+
data-fetch-users="/org7053/members.json"
120+
type="text"
121+
value="3" >
122+
</div>
123+
</div>
124+
125+
<button type="submit" class="crayons-btn">Save</button>
126+
</form>
127+
`;
128+
});
129+
130+
it('renders matching snapshot', async () => {
131+
await convertCoauthorIdsToUsernameInputs();
132+
expect(document.forms[0].innerHTML).toMatchSnapshot();
133+
});
134+
135+
it('works as expected', async () => {
136+
await convertCoauthorIdsToUsernameInputs();
137+
const input = document.querySelector(
138+
"input[placeholder='Add another...']",
139+
);
140+
input.focus();
141+
await userEvent.type(input, 'Bob,');
142+
143+
const hiddenField = document.querySelector(
144+
"input[name='article[co_author_ids_list]']",
145+
);
146+
expect(hiddenField.value).toBe('3, 2');
147+
});
148+
149+
it('can remove previously selected', async () => {
150+
await convertCoauthorIdsToUsernameInputs();
151+
const deselect = document.querySelector(
152+
'.c-autocomplete--multi__selected',
153+
);
154+
await userEvent.click(deselect);
155+
156+
const hiddenField = document.querySelector(
157+
"input[name='article[co_author_ids_list]']",
158+
);
159+
expect(hiddenField.value).toBe('');
160+
});
161+
});
162+
});

app/javascript/crayons/MultiSelectAutocomplete/MultiSelectAutocomplete.jsx

+6-1
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,12 @@ export const MultiSelectAutocomplete = ({
598598
className={`c-autocomplete--multi__wrapper${
599599
border ? '-border crayons-textfield' : ' border-none p-0'
600600
} flex items-center cursor-text`}
601-
onClick={() => inputRef.current?.focus()}
601+
onClick={(event) => {
602+
// Stopping propagation here so that clicks on the 'x' close button
603+
// don't appear to be "outside" of any container (eg, dropdown)
604+
event.stopPropagation();
605+
inputRef.current?.focus();
606+
}}
602607
>
603608
<ul id="combo-selected" className="list-none flex flex-wrap w-100">
604609
{allSelectedItemElements}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { h, render } from 'preact';
2+
import { UsernameInput } from '../../shared/components/UsernameInput';
3+
import { UserStore } from '../../shared/components/UserStore';
4+
import '@utilities/document_ready';
5+
6+
export async function convertCoauthorIdsToUsernameInputs() {
7+
const usernameFields = document.getElementsByClassName(
8+
'article_org_co_author_ids_list',
9+
);
10+
11+
const users = new UserStore();
12+
13+
for (const targetField of usernameFields) {
14+
targetField.type = 'hidden';
15+
const exceptAuthorId =
16+
targetField.form.querySelector('#article_user_id').value;
17+
const inputId = `auto${targetField.id}`;
18+
const fetchUrl = targetField.dataset.fetchUsers;
19+
const row = targetField.parentElement;
20+
21+
await users.fetch(fetchUrl).then(() => {
22+
const value = users.matchingIds(targetField.value.split(','));
23+
const fetchSuggestions = function (term) {
24+
return users.search(term, { except: exceptAuthorId });
25+
};
26+
27+
const handleSelectionsChanged = function (ids) {
28+
targetField.value = ids;
29+
};
30+
31+
render(
32+
<UsernameInput
33+
labelText="Add up to 4"
34+
placeholder="Add up to 4..."
35+
maxSelections={4}
36+
inputId={inputId}
37+
defaultValue={value}
38+
fetchSuggestions={fetchSuggestions}
39+
handleSelectionsChanged={handleSelectionsChanged}
40+
/>,
41+
row,
42+
);
43+
});
44+
}
45+
}
46+
47+
document.ready.then(() => {
48+
convertCoauthorIdsToUsernameInputs();
49+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
export class UserStore {
2+
constructor() {
3+
this.users = [];
4+
this.wasFetched = false;
5+
}
6+
7+
fetch(url) {
8+
const myStore = this;
9+
return new Promise((resolve, _reject) => {
10+
if (myStore.wasFetched) {
11+
resolve();
12+
} else {
13+
window
14+
.fetch(url)
15+
.then((res) => res.json())
16+
.then((data) => {
17+
myStore.users = data.reduce((array, aUser) => {
18+
array.push(aUser);
19+
return array;
20+
}, []);
21+
myStore.wasFetched = true;
22+
resolve();
23+
})
24+
.catch((error) => {
25+
Honeybadger.notify(error);
26+
resolve();
27+
});
28+
}
29+
});
30+
}
31+
32+
matchingIds(arrayOfIds) {
33+
const allUsers = this.users;
34+
const someUsers = arrayOfIds.reduce((array, idString) => {
35+
const aUser = allUsers.find((user) => user.id == idString);
36+
if (typeof aUser != 'undefined') {
37+
array.push(aUser);
38+
}
39+
return array;
40+
}, []);
41+
return someUsers;
42+
}
43+
44+
search(term, options) {
45+
options ||= {};
46+
const { except } = options;
47+
const allUsers = this.users;
48+
const results = [];
49+
for (const aUser of allUsers) {
50+
if (
51+
aUser.id != except &&
52+
(aUser.name.search(term) >= 0 || aUser.username.search(term) >= 0)
53+
) {
54+
results.push(aUser);
55+
}
56+
}
57+
return results;
58+
}
59+
}

0 commit comments

Comments
 (0)