Snipe-it: Drop down menus will Break if their are less than six elements

Created on 8 May 2020  路  27Comments  路  Source: snipe/snipe-it

Please confirm you have done the following before posting your bug report:

Describe the bug

Drop-down menus will display items for a period of time until it shows "The results could not be Loaded". This will prevent all drop down menus from work for a short period of time.

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'Login to the Demo on snipe-it website '
  2. Click on '"Create New" -> Asset . Click on Company Drop-down list '
  3. Scroll down to 'the bottom until you see "Loading More Results" and wait for 10 to 15 seconds'
  4. See error 'Menu items disappear and "The Results could not be loaded" appears. This will also break all other drop down menus on the app for a short period of time'.

Screenshots
error

Server (please complete the following information):

  • Snipe-IT Version Master(v4.9.2)(DEMO)
  • OS: [unknown]
  • Web Server: [unknown]
  • PHP Version

Desktop (please complete the following information):

  • OS: [Windows 10]
  • Browser [Chrome]
  • Version [latest]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Most helpful comment

I reverted back to an older version of all.js and it fixed the problem - it seems like the issue comes from select2.

image

The existing scroll callback works fine while the new one fails with the loop described above. I was able to short-cut this problem by hack fixing all.js from

}), this.$results.on("scroll", this.loadMoreIfNeeded.bind(this)) }, e.prototype.loadMoreIfNeeded = function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }, e.prototype.loadMore = function() {

to

}), this.$results.on("scroll", function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }) }, e.prototype.loadMore = function() {

I hope this makes sense - I'm certainly not a JS dev as you can probably tell :)

Update: Pull down latest from master - this issue is now fixed.

All 27 comments

I found this was because the browser send too many /selectlist?page= requests in a given amount of time.

Reference:
https://github.com/ParadoxGuitarist/jamf2snipe/issues/9
https://snipe-it.readme.io/reference#api-throttling

But I've no idea about why this happend, anybody can explain this?

Snipe-IT Documentation
Snipe-IT is a free, open source IT asset management system. Features include management of assets, users, licenses, accessories, consumables and components, as well as two-factor authentication, LDAP/AD syncing, and asset acceptance confirmation.

Also noticed this issue today after updating to the latest version in master and attempting to create a new Asset Model - I do have more than 6 items in a drop-down.
The issue appears to happen when I scroll to the bottom of a drop-down where Snipe-It would typically lazy-load more items:

image

This issue can be repro'd on the SnipeIT Demo site as well

I reverted back to an older version of all.js and it fixed the problem - it seems like the issue comes from select2.

image

The existing scroll callback works fine while the new one fails with the loop described above. I was able to short-cut this problem by hack fixing all.js from

}), this.$results.on("scroll", this.loadMoreIfNeeded.bind(this)) }, e.prototype.loadMoreIfNeeded = function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }, e.prototype.loadMore = function() {

to

}), this.$results.on("scroll", function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }) }, e.prototype.loadMore = function() {

I hope this makes sense - I'm certainly not a JS dev as you can probably tell :)

Update: Pull down latest from master - this issue is now fixed.

I reverted back to an older version of all.js and it fixed the problem - it seems like the issue comes from select2.

image

The existing scroll callback works fine while the new one fails with the loop described above. I was able to short-cut this problem by hack fixing all.js from

}), this.$results.on("scroll", this.loadMoreIfNeeded.bind(this)) }, e.prototype.loadMoreIfNeeded = function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }, e.prototype.loadMore = function() {

to

}), this.$results.on("scroll", function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }) }, e.prototype.loadMore = function() {

I hope this makes sense - I'm certainly not a JS dev as you can probably tell :)

it works! thank u!

Looks like snipe has noted the select2 fix on #7997 so it must be in-flight :)

Hello,

Exact same issue here with 405 method not allowed errors. Waiting for the fix.

The all.js edit workaround works for me too.
Nice finding Coaxke!

I've also edited the all.js file. However, it doesn't seem to be working in my environment. To be clear, we are talking about the all.js file in the ./public/js/build directory right?

Or do I need for each dropdown menu at least six items?

EDIT: I've changed the API limit from 120 to 500, so now I can edit and add a little bit more.

Hello ardvw,

you need to edit the all.js under the "dist" sub-container" if I remember well.

Cheers,

I reverted back to an older version of all.js and it fixed the problem - it seems like the issue comes from select2.
image
The existing scroll callback works fine while the new one fails with the loop described above. I was able to short-cut this problem by hack fixing all.js from
}), this.$results.on("scroll", this.loadMoreIfNeeded.bind(this)) }, e.prototype.loadMoreIfNeeded = function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }, e.prototype.loadMore = function() {
to
}), this.$results.on("scroll", function() { var e = t.contains(document.documentElement, this.$loadingMore[0]); !this.loading && e && this.$results.offset().top + this.$results.outerHeight(!1) + 50 >= this.$loadingMore.offset().top + this.$loadingMore.outerHeight(!1) && this.loadMore() }) }, e.prototype.loadMore = function() {
I hope this makes sense - I'm certainly not a JS dev as you can probably tell :)

it works! thank u!

Same here also running into the empty drop down issue. The all.js seems compressed (no newlines, etc), so had a hard time implementing the workaround. Any recommendations on how to patch this?
In the meantime I increased the API threshold, in app/Https/Kernel.php which helped a bit.
Could this be fixed in git, so that git pull will fix it?
Thanks

Exact same issue here with 405 method not allowed errors. Waiting for the fix.

This would not be causing "method not allowed" issues.

I'm legit a little baffled why our hosted customers aren't seeing this.

I'm still unable to reproduce this on the demo, but I definitely appreciate all the sleuthing here. I'm trying to see if bumping up our select2 library might help, but since I can't reproduce it, it makes it hard to tell. :-/

Looks like select2 hasn't released anything new recently, so their latest is the same version we're running. :(

We can't edit those libraries manually or they'll just get overwritten next time we update things.

Looks like this bit of select2 might be what's causing the issue:

https://github.com/select2/select2/blob/e5131d0cc8dffd98ba36a68f3d027bf79e763cb4/src/js/select2/dropdown/infiniteScroll.js#L43-L61

https://github.com/select2/select2/commit/d9260254c161e81cf09943dbb04114a7e5be1133#diff-6bfa161cfa52b7188ae7e42328aaadbe

Screen Shot 2020-07-07 at 9 00 20 AM

GitHub
Select2 is a jQuery based replacement for select boxes. It supports searching, remote data sets, and infinite scrolling of results. - select2/select2

That's so weird that the issue is no reproducible 馃槷 ; I just tried again in Demo on two browsers (no add-ins enabled) and could trigger it when attempting to create a new asset, selecting the model drop-down and scrolling to the bottom of the list (if dev tools are open you'll see a ton of requests).

Unfortunately if I try to downgrade select2 from before that patch I referenced above, we'll lose the accessibility fixes that were added afterwards. I need to figure out how best to approach this.

Has anyone posted an issue on the select2 GH issues about this?

I didn't raise an issue, sorry - I'm not a competent front-end dev and don't think I can explain the issue very well 馃槀

I'm going to tag in @uberbrady here to see if he can maybe offer a PR to select2, or find another workaround.

Well, the good news (ugh, sort of?) is that we're starting to see reports of this from a handful of customers as well, which gives us a little more to work with.

Okay, I can see what's going on here.

Snipe-IT is correctly returning {"pagination": {"more": false, ... in our response, but the Select2 JS is not respecting thefalse result correctly. It should stop requesting new pages.

Furthermore, if you were to have around 117 pages of results (the number where it broke for _me_ during my testing), the front-end JS code should be more resilient to HTTP errors such as these - I was thinking about maybe implementing some kind of exponential back-off. The Select2 list slowing down a little bit is much better than it completely breaking.

I'll dig further into the documentation for Select2 and update this issue with my plan(s) of attack.

Nope, it's not them - it's us.

We use a /selectlist URL to back all of our Select2 select lists. Each of those methods - which live on all of the main controllers we use - ends up returning results through a SelectlistTransformer to get output. But in our Select2 javascript initialization, we also specify a processResults callback element, which transforms the JSON returned by the server into the format that Select2 wants. In the current version of our javascript, we say:

pagination: {
                      more: "true" //(params.page  < data.page_count)
                    }

It's funny, I actually just noticed that @dmeltzer had noticed this too and also found it curious - I think he was right :)

So we're actually using the JS to transform the correct result into an incorrectly-infinite-scrolling result.

I'll dig around in the git history to see why we decided to do that. But I suspect that's where the problem lies.

Another thing I was thinking - this is a little bit of added complexity that we could probably just remove entirely - e.g. we could get rid of the processResults callback entirely, and just returning correctly-formatted results from our /selectlist methods directly - since they exist solely for the purpose of backing our usage of Select2. We could probably do that all within our SelectlistTransformer too - and then there's less code to go through and everything gets a little easier to troubleshoot in the future.

@uberbrady
make it so

There's a PR up against Master for these changes now - feel free to give it a try and let us know if it works for you.

(The PR is located here: https://github.com/snipe/snipe-it/pull/8227 )

I've merged that PR into master and it looks good - can anyone else confirm that this has resolved the issue?

Just pulled master down as well - looks good to me too!

Great - I'll close out this issue for now then.

Thanks again everyone for the great sleuthing, and thanks to @uberbrady for a quick fix.

rdj_thanks

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Supsuop picture Supsuop  路  4Comments

memtech3 picture memtech3  路  4Comments

ArchdukeNavaron picture ArchdukeNavaron  路  4Comments

Rungea96 picture Rungea96  路  4Comments

snipe picture snipe  路  3Comments