-
-
Notifications
You must be signed in to change notification settings - Fork 776
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
GITC-389 Remove duplicates from explore page #9695
Conversation
app/assets/v2/js/grants/index.js
Outdated
@@ -320,8 +320,13 @@ if (document.getElementById('grants-showcase')) { | |||
if (!append_mode) { | |||
vm.grants = []; | |||
} | |||
|
|||
const previouslyLoadedIds = vm.grants.map(grant => grant.id); |
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.
Consider: moving this to the vue model to avoid creating the mapping every time we call fetchGrants()
app/assets/v2/js/grants/index.js
Outdated
getGrants.grants.forEach(function(item) { | ||
vm.grants.push(item); | ||
if (!previouslyLoadedIds.includes(item.id)) { |
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.
If previouslyLoadedIds
was stored as an object against the viewModel we could make this check O(1) instead of O(n) and avoid the need to map vm.grants
into previouslyLoadedIds
each call...
if (!previouslyLoadedIds.includes(item.id)) { | |
if (!vm.previouslyLoadedIds[item.id]) { | |
vm.previouslyLoadedIds[item.id] = item; |
82d7c49
to
9b69c1d
Compare
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.
LGTM! 🚀
Description
This is a quick fix for resolving the bug in GITC-389. When in append mode (the mode triggered from scrolling), before adding new grants received from the server, it will check to see if it already exists. A more robust fix can be included in the server, that will be added as a new ticket.
Refers/Fixes
GITC-389
Testing
Tested locally