Fix quoting and url-encoding

[kroussou]

Description

[Describe what this change achieves]

Straightforward fix of quoting and url-encoding issues which prevent reports from being generated or cause reports being generated with wrong time range (and also produce rison decoder error: missing ':' errors)

Ported from opendistro-for-elasticsearch/kibana-reports#394

Issues Resolved

[List any issues this PR will resolve]
Related-to: opendistro-for-elasticsearch/kibana-reports#355
Fixes: #88, #61

Check List

• New functionality includes testing.
  • All tests pass, including unit test, integration test and doctest
• New functionality has been documented.
  • New functionality has javadoc added
  • New functionality has user manual doc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[zhongnansu]

Hi, thx for contributing. I saw one of the github flow is failing, could you fix that？ @kroussou

[kroussou]

@zhongnansu Hi! Thanks for looking at my PR.
Based on logs failing check is not related to my PR. Linkchecker complains about README.md and LICENSE.md files which I didn't touch. Either I messed something in logs or check itself is misconfigured

Errors in README.md
40
✗ file:///github/workspace/reports-scheduler/README.md (Invalid file URI: file:///github/workspace/reports-scheduler/README.md)
41
✗ file:///github/workspace/LICENSE.txt (Invalid file URI: file:///github/workspace/LICENSE.txt)

[codecov-commenter]

Codecov Report

Merging #153 (7939f00) into main (dc55f62) will increase coverage by 2.59%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #153      +/-   ##
============================================
+ Coverage     62.42%   65.01%   +2.59%     
+ Complexity      309      292      -17     
============================================
  Files           101       96       -5     
  Lines          4484     4053     -431     
  Branches        681      632      -49     
============================================
- Hits           2799     2635     -164     
+ Misses         1517     1277     -240     
+ Partials        168      141      -27     

Flag	Coverage Δ
dashboards-reports	73.30% <100.00%> (ø)
reports-scheduler	56.69% <ø> (+3.27%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../components/main/report_details/report_details.tsx	76.14% <100.00%> (ø)
...n/org/opensearch/reportsscheduler/model/RestTag.kt	100.00% <0.00%> (ø)
...nsearch/reportsscheduler/ReportsSchedulerPlugin.kt	100.00% <0.00%> (ø)
...duler/resthandler/ReportInstancePollRestHandler.kt
...eportsscheduler/action/PollReportInstanceAction.kt
.../reportsscheduler/settings/LegacyPluginSettings.kt
...portsscheduler/model/PollReportInstanceResponse.kt
...eportsscheduler/model/PollReportInstanceRequest.kt
.../reportsscheduler/model/ReportDefinitionDetails.kt	75.40% <0.00%> (+1.21%)	⬆️
...search/reportsscheduler/settings/PluginSettings.kt	72.13% <0.00%> (+1.64%)	⬆️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc55f62...7939f00. Read the comment docs.

[kroussou]

@zhongnansu I see that all checks are green now so you may proceed with the review

[zhongnansu]

1. I feel like this is more of a hardcode solution that doesn't solve a generic issue. Take a look at what else that needs to be decoded. It's not only %2F -> /, e.g. + will also need to be decoded, that's why we have issue with future dates
   https://www.elastic.co/guide/en/elasticsearch/reference/current/date-math-index-names.html

2. After step 1 , the Today time range selector will still have issue. Because we are using dateMath.parse() to convert ES datamatch expression to Moment object. I did some research, if you add an option roundUp: true to the parse function, on toDate, we can get expected behavior same as Kibana

Can you try implementing those? Then I think we can get a complete solution for the 2 issues you listed

[kroussou]

@zhongnansu Thanks for your input.
For step 1 I think using decodeURIComponent instead of simple replace should do the trick. I've made some test yesterday and everything looks good so far. I'm going to commit this change shortly.
For step 2 I need to do more research though

[zhongnansu]

@zhongnansu Thanks for your input. For step 1 I think using decodeURIComponent instead of simple replace should do the trick. I've made some test yesterday and everything looks good so far. I'm going to commit this change shortly. For step 2 I need to do more research though

@kroussou Thanks for making the change.
For step 2, this is the parse function I am talking about. https://github.com/opensearch-project/dashboards-reports/blob/371f1d18855de49688379634fa867f3f8eaef8e4/dashboards-reports/public/components/context_menu/context_menu_helpers.js#L150

To better describe the issue. Taking today for example. After all the unexcape/decode，finally we'll get a time rage of

fromDate: `now/d`
toDate: `now/d`

then dateMath.parse() into ISO will result in the same timestamp for "from" and "to", which is the begining of that date. This is not expected. If you take a look at the timestamp in Discover page after you select "Today". It is actually converting the toDate to the end of today, which is a "roundUp". And the fromDate is being rounded down to the begining of that date, which is the default behaviour of dateMath.parse()

I believe Kibana itself is also using the same conversion, given what we see in the Discover timestamp. If you check the usage of that library in Kibana itself, you can find many reference of using roundUp for toDate

[kroussou]

I believe Kibana itself is also using the same conversion, given what we see in the Discover timestamp

Looks so. At least time ranges now match between UI and reports.
I had doubts that it's also true for some weird expressions for toDate like now-1h/d, now/d-1h, 2021-10-04T10 (incomplete absolute date). However I see that UI shows the same number of events as in CSV