-
Notifications
You must be signed in to change notification settings - Fork 1
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
[EQK-29] Added Query Statistic Page #37
base: develop
Are you sure you want to change the base?
Conversation
action="foundation.dialog" | ||
target=".eqk-ui" | ||
action="foundation.link" | ||
href="/mnt/overlay/commerce/gui/content/v2/catalogs/createcatalogwizard.html${requestPathInfo.suffix}" |
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.
Something's wrong with link, yeah?) Why "create catalog wizard"?
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.
sorry, it's a typo)
text="Settings..."> | ||
<data jcr:primaryType="nt:unstructured" | ||
nesting="hide" | ||
src.url="/mnt/overlay/etoolbox-query-kit/console/dialogs/settings.html"/> | ||
</settings> | ||
<statistic |
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.
(spelling) It is always "statistics"
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.
fixed, thank you!
@@ -0,0 +1,95 @@ | |||
<sly data-sly-use.queryStatisticModel="com.exadel.etoolbox.querykit.core.models.query.QueryStatisticModel"></sly> |
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.
(spelling) Please rename to "...statistics" everywhere
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.
All changed
${queryStatisticModel.indexesByUsage[index]} | ||
</sly> | ||
|
||
<!--/*<ul data-sly-list.index="${queryStatisticModel.indexesByUsage}">*/--> |
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.
Do we need this part?)
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.
no, it was just for demo. Deleted unused code
private LinkedHashMap<String, List<QueryStatisticModel.QueryInfo>> queriesByIndexSorted = new LinkedHashMap<>(); | ||
|
||
@PostConstruct | ||
protected void init() throws IOException { |
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.
private
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.
fixed
Set<ObjectName> names = server.queryNames(new ObjectName("org.apache.jackrabbit.oak:type=QueryStats,*"), null); | ||
return names.iterator().next(); | ||
} catch (IOException | MalformedObjectNameException | NoSuchElementException e) { | ||
return null; |
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.
Need to add logging here
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.
added
} | ||
|
||
private String getIndexId(String value) { | ||
if (value.contains("(") && value.contains(")")) { |
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.
Please use Constants.OPENING_BRACKET, etc. (and please review other places when existing constants could be used
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.
Done
} | ||
} | ||
|
||
public static class QueryInfo { |
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.
- I'd rather extracted into a separate class file
- Please refactor to use Lombok. It's Lombok throughout this project
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.
Done
<clientlibs | ||
jcr:primaryType="nt:unstructured" | ||
sling:resourceType="granite/ui/components/coral/foundation/includeclientlibs" | ||
categories="[cq.common.wcm,cq.listview.coral.columns.personalization,cq.siteadmin.admin.page.row,cq.gui.coral.common.admin.references,cq.screens.dcc,etoolbox-query-kit.queryStatistic]"/> |
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.
Do we need all these categories here? They look like too much
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.
Optimized)
@@ -0,0 +1,6 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
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.
Let us move to the main clientlibs folder. The rest of css/js in one place. I frankly see no big reason fo these two rules to be in another
No description provided.