Fix Report Creation after Report Definition Creation

[davidcui1225]

Signed-off by: David Cui davidcui@amazon.com

Description

Fixes on-demand report creation after a new report definition is created.

Issues Resolved

[List any issues this PR will resolve]

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.

[codecov-commenter]

Codecov Report

Merging #196 (a1a9780) into main (93d885c) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               main     #196   +/-   ##
=========================================
  Coverage     62.60%   62.60%           
  Complexity      297      297           
=========================================
  Files            99       99           
  Lines          4190     4190           
  Branches        661      661           
=========================================
  Hits           2623     2623           
  Misses         1424     1424           
  Partials        143      143           

Flag	Coverage Δ
dashboards-reports	72.74% <ø> (ø)
reports-scheduler	52.83% <0.00%> (ø)

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

Impacted Files	Coverage Δ
...h/reportsscheduler/action/ReportInstanceActions.kt	70.21% <0.00%> (ø)

---

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 93d885c...a1a9780. Read the comment docs.

[zhongnansu]

I wonder when there will be a case that reportDefinition.Delivery is not null but the configId is empty? Is it a coner case? @davidcui1225

[davidcui1225]

I wonder when there will be a case that reportDefinition.Delivery is not null but the configId is empty? Is it a coner case? @davidcui1225

If reportDefinition.delivery is not null then the front-end validation will fail since it won't let an empty configId be created. https://github.com/opensearch-project/dashboards-reports/blob/93d885c641c8c7fc8beeb081bee6ffd30b44f73b/dashboards-reports/public/components/report_definitions/utils/utils.tsx#L109

[joshuali925]

I wonder when there will be a case that reportDefinition.Delivery is not null but the configId is empty? Is it a coner case?

if configId is empty then backend should not try to send notifications anyways. This PR is needed because I thought reportDefinition.Delivery will be null if user did not configure notifications, but seems it will be an object with empty fields

[zhongnansu]

I wonder when there will be a case that reportDefinition.Delivery is not null but the configId is empty? Is it a coner case?

if configId is empty then backend should not try to send notifications anyways. This PR is needed because I thought reportDefinition.Delivery will be null if user did not configure notifications, but seems it will be an object with empty fields

I agree that reportDefinition.Delivery should be null if user doesn't configure notification. An empty object is confusing, we should look into how to fix that.