sigmah
sigmah copied to clipboard
sigmah-dev
[WIP] Search Engine (issue 535)
Good frequency of small commits, keep with this habit ! I makes following and understanding easier !
Two general comments:
- please take a look at the Codacy report on your pull request to improve its quality
- your commits have actually broken Contribution rule number one : each commit must be linked to one and only one Mantis issue : http://wiki.sigmah.org/doku.php?id=contributorguide:contributionrules#make_your_work_easy_to_merge
Could you adjust your future work to improve those two issues ?
Good work so far ! Continue like that !
Apologies for forgetting to put the issue number each time! The codacy report mostly suggests that I clean up my unused imports. At present I am keeping them just in case I might need to import some stuff again, and I'll clean them up as soon as I'm sure I don't require them at all. And I'll soon modify my pom.xml file as well and remove local settings as you mentioned. Thank you!
In my latest commit ( https://github.com/sigmah-dev/sigmah/pull/97/commits/8dd81b444321a9d794f7c199b8d6f385857695d3 ), most projects are opening out on clicking the corresponding search result, but some are facing permission issues. Need to investigate what are the permissions preventing those specific projects from opening out.
In the commit https://github.com/sigmah-dev/sigmah/pull/97/commits/dd095d67ee68ca8e9f0f8e23f4e5a74d17147367, I have added an auto index job which does a solr full data import. This job is initialized when the dashboard first loads ( this is a temporary solution just to test whether the job works or not ), and keeps firing every 10 minutes.
https://github.com/sigmah-dev/sigmah/pull/97/commits/0604656280e0fc5eb3c3e2ff4c253870a7b4e0e6 : Should be global 'solr' core url.
https://github.com/sigmah-dev/sigmah/pull/97/commits/bec9e807d9a725b308c9da154574b9e68489aa5d: I have added a column in the table organization, hence added a migration sql file as mentioned here: http://wiki.sigmah.org/doku.php?id=contributorguide:contributionrules. I hope there is nothing more to change.
It seems there is a jdk version error, i.e I am compiling with JDK 1.8 and the builder is running tests with JDK 1.6 or 1.7 : https://stackoverflow.com/questions/23249331/java-unsupported-major-minor-version-52-0 That is the reason it is not able to pass the tests.
By "rudimentary" files results filtering, I mean that only those files which the currently logged in user is an author of are being shown in the search results. On clicking these results, the file successfully downloads. This is the best permission I am able to implement for now ( and is partially useful at the same time ), due to the complexity of the code. For future, a better permission has to be implemented.
Hi @osarrat @niksj , I need to write a brief description / gist here as part of the final evaluation. Can I get rights for the same? Currently I am unable to write a description.
Well, for this one i'm 50/50 about integration.
At the first sight, it seems that it's a good basis with not so much impacts, which is nice but on the other hand, while the development has been focused on building a viable POC (which is fine), non-working/limit cases seems not sufficiently considered (e.printStackTrace() instead of exception management). Also, I know that a big issue about search is about keeping the index and the database in sync (involving transaction management to only commit changes in the index if commit in the database has passed) and I know solr is very fast but sometimes corrupts its index, so being able to reset/clear the index and rebuild it (reindexing the whole database) should be available as a feature (and I didn't see it, but I only had a quick review).
Seems a good start with things in the right place through. I think a developer who would have to implement this feature should seriously consider continuing this PR, it may be faster than starting from nothing.
Thanks @numero-six for this review !
So, if I understand you correctly, your reluctance mainly come from functional issues like a lack of reset button, or improvable error messages. But you consider this piece of work as valuable enough to start to have a search engine in Sigmah. The good thing is that this search engine feature has been developed so that it can be easily enabled or disabled to some user profiles by a single global privilege. So, in my opinion, if we consider the risk to lose this big amount of work because too different from the master, I would recommend to merge this PR and informing users that it is still an early feature. Does it seem a good trade-off in your opinion @numero-six ?