fergiemcdowall/norch

pageSize and offset snafu?

eklem opened this issue ยท 18 comments

That sounds like a bug :)

eklem commented

My guess is that it's in search-index since mostly what norch is doing is to pass stuff back and forth?

Yes, most likely a bug in search-index

There are now tests for offset and pagesize in search-index and norch, and everything seems to be passing.

I have squashed some minor, slightly related bugs lately, so maybe if you update to 0.8.7 things will start working. If not, I can take another look at it.

eklem commented

I'll do testing on the newest stuff, @fergiemcdowall

eklem commented

Have you pushed a norch-version that depends on the latest search-index?

eklem commented

published I mean, not pushed

Yes- Norch is now up to date with the latest (today's) search-index

Ok- can you pm me the dataset, or a snapshot?

eklem commented

I'll make a test-case, and get to re-test it cleanly in the same go!

eklem commented

If you do:

npm install search-index-indexer
node node_modules/search-index-indexer/index.js -c https://raw.githubusercontent.com/eklem/search-index-indexer/master/config.json -d https://raw.githubusercontent.com/eklem/dataset-vinmonopolet/master/dataset-vinmonopolet-sparkling.str
mkdir norch-index
mv ./si/ ./norch-index/data/
norch -p 3030 -l silly -i norch-index

Then test these two links:
http://localhost:3030/search?q={"query":{"AND":{"Varenavn":["champagne"]}},"pageSize":"3"}
http://localhost:3030/search?q={"query":{"AND":{"Varenavn":["champagne"]}},"pageSize":"3","offset":"1"}

Have reproduced the error by doing

npm install norch
(... start norch)
curl https://raw.githubusercontent.com/eklem/dataset-vinmonopolet/master/dataset-vinmonopolet-sparkling.str > data.json
curl -X POST -d @data.json http://localhost:3030/add

Looking at it now...

This one is a real "doh!"- The values of offset and pageSize have to be integers

So http://localhost:3030/search?q={"query":{"AND":{"Varenavn":["champagne"]}},"pageSize":3,"offset":1} works

It would be good if the library was a bit more forgiving and allowed "1" to be interpreted as 1

eklem commented

I'll test!

Need to push out the fix for doing stuff like offset: "2" onto npm- just doing that now. (offset: 2 works fine)

eklem commented

Tested and working well here to ๐Ÿ˜ƒ

good! :)