Skip to content
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

SOLR-15853 Solr paramset impl #594

Closed
wants to merge 13 commits into from

Conversation

betulince
Copy link
Contributor

https://issues.apache.org/jira/browse/SOLR-15853

Description

A screen where paramset can be created has been added to the admin tool. You can set a new paramset on this screen and bring this paramset. When the get paramset button is pressed, you can see all the paramsets that this collection has. You can only display it by typing a paramset whose name you know in the input field.

In addition, a new section has been added to the query screen, enabling the use of the paramset desired to be used in this field. Multiple selection is also possible here. If you select the paramsets you want to use and say execute query, you will see that this paramset is added to the url as useParam=paramset_name and returns the correct results.
I applied the films example in this link to test it, it gave the expected results.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

@betulince
Copy link
Contributor Author

You can also check this newly added feature from the screenshots I have added.

Screenshot from 2022-02-04 16-08-15

Screenshot from 2022-02-04 16-05-55

Screenshot from 2022-02-04 16-06-50

Screenshot from 2022-02-04 16-07-47

Screenshot from 2022-02-04 16-08-03

@betulince betulince changed the title Solr paramset impl Solr paramset impl SOLR-15853 Feb 4, 2022
@betulince betulince changed the title Solr paramset impl SOLR-15853 SOLR-15853 Solr paramset impl Feb 4, 2022
solr/webapp/web/js/angular/controllers/paramsets.js Outdated Show resolved Hide resolved
@@ -209,7 +247,7 @@ solrAdminApp.controller('QueryController',
$location.search(key, adminParams[key]);
}
var currentUrl = $location.$$url;
if (previousUrl === currentUrl) { //if the query send with same parameters the query should be executed
if (previousUrl === currentUrl) { //if the query send with same parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old comment looks better than new one. please return back to old

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@betulince I don't think this comment was addressed.

solr/webapp/web/partials/paramsets.html Outdated Show resolved Hide resolved
@@ -23,6 +23,18 @@
</label>
<input type="text" ng-model="qt" id="qt" placeholder="/select" value="/select" title="Request handler in solrconfig.xml.">

<div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend that field should be optional. If the core has paramsets then it should be shown. otherwise it should not be shown..
But it's up to you

@epugh
Copy link
Contributor

epugh commented Feb 4, 2022

This is Super Exciting! I am going to go through and test this, but probably not till Monday.

One thing is, I wonder, now that we are exposing this in the UI, I wonder if the best name is ParamSets? I am going to start a thread on the Dev list about the name of this feature. It may amount to some bikeshedding, or now may be the time to come up with a better name to describe configuring a Solr end point. If you are not on the dev list, you might want to join @betulince

@betulince
Copy link
Contributor Author

betulince commented Feb 4, 2022 via email

@epugh
Copy link
Contributor

epugh commented Mar 14, 2022

Can't wait to see what you added...

FYI, over the weekend I needed a better "multiselect", and someone had recommended selectize.js, so I rolled that in as part of this #745

It supposedly has drag and drop ordering, though I haven't figured that out!

@epugh
Copy link
Contributor

epugh commented Mar 14, 2022

looks like angular-ui-select is also either based or inspired by selectize.js!

@epugh
Copy link
Contributor

epugh commented Mar 14, 2022

is package.json actually integrated into the build? I think we've never had one before...

@betulince
Copy link
Contributor Author

hey! I deleted package.json and package-lock.json. They are not related with the improvement, I just added while I was trying some things :) And I didn't send the improvement on paramset screen yet. I'll send it in some hours, I am just checking and doing final touches. Sorry for late reply it's just 3 am in the morning in turkey :) I am excited for the results!!

@betulince
Copy link
Contributor Author

Improvement done. It works correctly according to my tests. Instead of copying the placeholder, it made more sense to show a sample json and have it copied...
On the query screen, you can change the order of paramsets by drag&drop. And it goes directly to the UI with the chosen order.

Copy link
Contributor

@risdenk risdenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have comments on the functionality of this. I think there is one existing review comment not addressed.

#745 and this PR both try to pull in a UI framework for selection. @epugh did you get to see which one is better to go with?

I also saw @epugh left a video comment in https://issues.apache.org/jira/browse/SOLR-15853?focusedCommentId=17491578&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17491578 - I didn't watch it but it seems like @epugh would be a good one to take another look.

@@ -209,7 +247,7 @@ solrAdminApp.controller('QueryController',
$location.search(key, adminParams[key]);
}
var currentUrl = $location.$$url;
if (previousUrl === currentUrl) { //if the query send with same parameters the query should be executed
if (previousUrl === currentUrl) { //if the query send with same parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@betulince I don't think this comment was addressed.

@betulince
Copy link
Contributor Author

I don't have comments on the functionality of this. I think there is one existing review comment not addressed.

#745 and this PR both try to pull in a UI framework for selection. @epugh did you get to see which one is better to go with?

I also saw @epugh left a video comment in https://issues.apache.org/jira/browse/SOLR-15853?focusedCommentId=17491578&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17491578 - I didn't watch it but it seems like @epugh would be a good one to take another look.

I just deleted the comment. @epugh maybe you can give another look at the code!

@epugh
Copy link
Contributor

epugh commented Jun 28, 2022

Finally getting back to this (I am on day 2 of a three day drive across america! ;-) ).... I don't know WHY I thought overwrite was a parameter???? Going to remove it...

@epugh
Copy link
Contributor

epugh commented Jun 29, 2022

I did a bunch of work, but for some reason couldn't push to the existing branch, so I had to create a new pull request at #923, so closing this one in favour of the new one!

@epugh epugh closed this Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants