4791 add report versioning #5533
Conversation
github-actions
bot
added
enhancement
Bundle size differenceComparing this PR to
|
| version: report.version, | ||
| code: report.code, | ||
| })?; | ||
| }; |
There was a problem hiding this comment.
Hmm not very happy with logic here, do you mind doing a self review of the changes and existing logic in this file and seeing how it can/should be simplified ?
There was a problem hiding this comment.
Hi Andrei,
I was wondering about just upserting all reports regardless of if they were older versions than the one existing on the server.
Is this what you're thinking? Or are you thinking refactoring but keeping the current logic?
| return 0; | ||
| }; | ||
|
|
||
| export const useLatestReportList = ({ |
There was a problem hiding this comment.
Front end should be report version agnostic, this should be back end logic
Also add unique is_custom identifier to id
including custom report options
for better performance
| pub fn find_one_by_code_and_version( | ||
| &self, | ||
| code: &str, | ||
| version: &str, | ||
| ) -> Result<Option<ReportRow>, RepositoryError> { | ||
| let result = report_dsl::report | ||
| .filter(report_dsl::code.eq(code)) | ||
| .filter(report_dsl::version.eq(version)) | ||
| .first(self.connection.lock().connection()) | ||
| .optional()?; | ||
| Ok(result) | ||
| } | ||
|
|
There was a problem hiding this comment.
Removing as not anymore
| pub fn mock_report_g() -> ReportRow { | ||
| inline_init(|r: &mut ReportRow| { | ||
| r.id = "mock_report_g".to_string(); | ||
| r.version = "2.3.0".to_string(); | ||
| r.is_custom = true; | ||
| r.code = "report_with_custom_option".to_string(); | ||
| }) | ||
| } | ||
|
|
||
| pub fn mock_report_h() -> ReportRow { | ||
| inline_init(|r: &mut ReportRow| { | ||
| r.id = "mock_report_h".to_string(); | ||
| r.version = "2.3.1".to_string(); | ||
| r.is_custom = false; | ||
| r.code = "report_with_custom_option".to_string(); | ||
| }) | ||
| } | ||
|
|
||
| pub fn mock_report_i() -> ReportRow { | ||
| inline_init(|r: &mut ReportRow| { | ||
| r.id = "mock_report_i".to_string(); | ||
| r.version = "2.8.2".to_string(); | ||
| r.is_custom = true; | ||
| r.code = "report_with_custom_option".to_string(); | ||
| }) | ||
| } | ||
|
|
||
| pub fn mock_report_j() -> ReportRow { | ||
| inline_init(|r: &mut ReportRow| { | ||
| r.id = "mock_report_j".to_string(); | ||
| r.version = "3.0.1".to_string(); | ||
| r.is_custom = false; | ||
| r.code = "report_with_custom_option".to_string(); | ||
| }) | ||
| } |
There was a problem hiding this comment.
Added extra cases to test for prioritising is_custom reports
| let reports_to_show_meta_data = report_filter_method( | ||
| repo.query_meta_data(Some(filter.clone()), None)?, | ||
| app_version, | ||
| ); |
There was a problem hiding this comment.
Added querying all for metadata first with a sub selection of code, version, id, and is_custom. I thought this would help if our report list gets long and we aren't wanting to query all reports and filter out later.
Later we can collect the ids, and query all report data.
| Ok(reports) | ||
| } | ||
|
|
||
| fn report_filter_method( |
There was a problem hiding this comment.
I have a feeling this logic could be more concise. However, after taking some time to think about it, I figure it is not worth spending time on now given we are only passing around report_meta_data
| }; | ||
|
|
||
| use crate::{ | ||
| diesel_macros::{apply_equal_filter, apply_sort_no_case}, | ||
| schema_from_row, FormSchema, FormSchemaRow, | ||
| }; | ||
| use crate::{EqualFilter, Pagination, Sort, StringFilter}; |
There was a problem hiding this comment.
I have removed pagination as we won't use it for these queries.
Fixes #4791
👩🏻💻 What does this PR do?
Two parts:
There is a unique case which could casue issues where if a db has a report version later than the app version, you wouldn't be able to upload older report versions. But I don't think we need to worry about that.
💌 Any notes for the reviewer?
You can test this report with this database which includes a number of report versions and is_custom combinations:
central_test.sqlite.zip
🧪 Testing
📃 Documentation