feat: provision to enable/disable prepared report from permission for page and report document #6831
Conversation
… page and report document
rohitwaghchaure
force-pushed
the
provision_to_enable_prepared_report_from_permissions_page
branch
from
581849a
to
38b47ef
Compare
|
it should be enabled by default. "Disable Prepared Report"? Also will you add execution time and progress bar? |
|
Also will you add execution time and progress bar? |
frappe/core/doctype/report/report.py
Outdated
| if not hasattr(frappe.local, 'is_prepared_report'): | ||
| frappe.local.is_prepared_report = {} | ||
|
|
||
| if not report in frappe.local.is_prepared_report: |
There was a problem hiding this comment.
I may be unclear on how the frappe.local object works, but if a report's prepared_report field is updated more than once in a session (either manually or by reaching the threshold), wouldn't this check not allow the frappe.local object to be modified, and always return the state it was initialised with?
| @@ -19,7 +20,11 @@ def get_custom_roles(self): | |||
| roles = self.get_standard_roles() | |||
|
|
|||
| self.set('roles', roles) | |||
|
|
|||
|
|
|||
| if self.report: | |||
There was a problem hiding this comment.
Should this be in a different function? Otherwise it's tied to get_custom_roles calls, which seems unrelated.
There was a problem hiding this comment.
Added in new function check_prepared_report_disabled
| @@ -50,6 +55,9 @@ def set_custom_roles(self): | |||
| else: | |||
| frappe.get_doc(args).insert() | |||
|
|
|||
| if self.report: | |||
There was a problem hiding this comment.
Same as above: should this be in a different function?
|
|
||
| if self.report: | ||
| prepared_report = is_prepared_report(self.report) | ||
| self.set('enable_prepared_report', prepared_report) |
There was a problem hiding this comment.
Benefit of doing self.set(...) vs just self.enable_prepared_report = is_prepared_report(self.report)?
rohitwaghchaure
force-pushed
the
provision_to_enable_prepared_report_from_permissions_page
branch
3 times, most recently
from
1413423
to
2d3afa8
Compare
rohitwaghchaure
force-pushed
the
provision_to_enable_prepared_report_from_permissions_page
branch
from
2d3afa8
to
b5f1cb0
Compare
|
@rmehta Added execution time and progress bar in the report |
rohitwaghchaure
force-pushed
the
provision_to_enable_prepared_report_from_permissions_page
branch
from
b5f1cb0
to
f17b476
Compare
rohitwaghchaure
force-pushed
the
provision_to_enable_prepared_report_from_permissions_page
branch
from
f17b476
to
c573cf1
Compare
| @@ -193,3 +193,14 @@ def _format(parts): | |||
| @Document.whitelist | |||
| def toggle_disable(self, disable): | |||
| self.db_set("disabled", cint(disable)) | |||
|
|
|||
| @frappe.whitelist() | |||
| def is_prepared_report_disabled(report): | |||
There was a problem hiding this comment.
Seems like the function is just a get_value call now. Might as well remove this function and do the call inline in the other file.
| }); | ||
| }); | ||
|
|
||
| return this._load_script; | ||
| } | ||
|
|
||
| setup_progress_bar() { | ||
| let i = 0; |
| const message = __('For comparison, use >5, <10 or =324. For ranges, use 5:10 (for values between 5 & 10).'); | ||
| this.page.footer.removeClass('hide').addClass('text-muted text-center').html(`<p>${message}</p>`); | ||
| const execution_time_msg = __('Exection Time: {0} sec.', [this.execution_time || 0.1]); |
There was a problem hiding this comment.
period (.) not required after sec
|
|
||
| this.interval = setInterval(function() { | ||
| i += 1; | ||
| frappe.show_progress(__('Loading Data'), i, execution_time); |
rohitwaghchaure
force-pushed
the
provision_to_enable_prepared_report_from_permissions_page
branch
from
eefb329
to
36327f2
Compare
Added progress bar
Added report execution time in the report