-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/#343 performance of typeahead/select2 with thousands of users #356
Feature/#343 performance of typeahead/select2 with thousands of users #356
Conversation
searching with a server-method
there are now 2 groups: participants and other user
it is now possible to add additional participants and search not only username, but also profile name of participants
search within users' longname is possible only in trusted environment
added a 50 ms delay before the search + free text user is now in the dropdown list
the delay for an ajax-request ist now 0 ms for e2e tests
+ removed additional paticipants from action item responsibles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please check the code climate findings and fix them: https://codeclimate.com/github/4minitz/4minitz/pull/356
client/templates/topic/topicEdit.js
Outdated
children: remainingUsers | ||
}]; | ||
let delayTime = Meteor.settings.public.isEnd2EndTest ? 0 : 50; | ||
|
||
selectResponsibles.select2({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function call is almost identical to the one in topicInfoItemEdit. The only difference appears to be a bool flag given to the responsiblesSearch
meteor method (third parameter. It's true
here and false
in topicInfoItemEdit.js).
Can you please try to factor this out into a separate function and call that function from both locations?
client/templates/topic/topicEdit.js
Outdated
} else { | ||
Minutes.formatResponsibles(responsibleUser, 'username', responsibleUser.profile); | ||
} | ||
selectResponsibles.append('<option value="' + responsibleId + '" selected="selected">' + responsibleUser.fullname + '</option>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes us open for a possible XSS attack, e.g. if the full name of a responsible user contains HTML/JavaScript. @felixble was already involved in safely handling raw html rendering, he may know more about how to mitigate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we had similar issues with our ConfirmationDialog which could be solved by using the blaze template engine.
Have a look at the function Blaze.renderWithData(templateOrView, data, parentNode, [nextNode], [parentView]).
I would suggest to prepare a simple array of responsibles containing the required data (responsibleId
and fullname
). Then you could provide a meteor template which consumes such an array and prints for each item an option
-Element. Then you can use the function Blaze.renderWithData
by passing the template, the data as well as the element selectResponsibles
as parentNode
. You can access the template with its name just like this: Template['template-name']
.
The meteor docs says "If [the nextNode is] not provided, the template will be inserted as the last child of parentNode." (see Blaze.renderWithData So it should have the same effect as appending strings like you did before. Fortunately, the blaze engine takes care about sanitizing the raw html 👍
Adding new user to a DB was done with a following method: