-
Notifications
You must be signed in to change notification settings - Fork 16
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
6650 rename requisition line transformation and expose plugin data query to plugins #6684
base: 5837-requisition-line-column-and-plugin-data
Are you sure you want to change the base?
Changes from 3 commits
5fb1eec
0a2f461
57a6a5f
5059486
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. | ||
|
||
export type EqualFilter<T> = { equal_to?: T, not_equal_to?: T, equal_any?: Array<T>, equal_any_or_null?: Array<T>, not_equal_all?: Array<T>, is_null?: boolean, }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. | ||
import type { EqualFilter } from "./EqualFilter"; | ||
|
||
export type PluginDataFilter = { id?: EqualFilter<string>, plugin_code?: EqualFilter<string>, related_record_id?: EqualFilter<string>, data_identifier?: EqualFilter<string>, store_id?: EqualFilter<string>, }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. | ||
import type { RequisitionLineRow } from "./RequisitionLineRow"; | ||
import type { RequisitionRow } from "./RequisitionRow"; | ||
|
||
export type TransformRequestRequisitionLineInput = { requisition: RequisitionRow, lines: Array<RequisitionLineRow>, }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. | ||
import type { PluginDataRow } from "./PluginDataRow"; | ||
import type { RequisitionLineRow } from "./RequisitionLineRow"; | ||
|
||
export type TransformRequestRequisitionLineOutput = { transformed_lines: Array<RequisitionLineRow>, plugin_data?: Array<PluginDataRow>, }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import { | |
TextArea, | ||
useAuthContext, | ||
useNavigate, | ||
usePluginProvider, | ||
useToggle, | ||
} from '@openmsupply-client/common'; | ||
import { DraftRequestLine } from './hooks'; | ||
|
@@ -71,6 +72,7 @@ export const RequestLineEdit = ({ | |
}: RequestLineEditProps) => { | ||
const t = useTranslation(); | ||
const navigate = useNavigate(); | ||
const { plugins } = usePluginProvider(); | ||
const { isOn, toggle } = useToggle(); | ||
const [anchorEl, setAnchorEl] = React.useState<null | HTMLElement>(null); | ||
const { store } = useAuthContext(); | ||
|
@@ -83,6 +85,9 @@ export const RequestLineEdit = ({ | |
?.sort((a, b) => a.name.name.localeCompare(b.name.name)) | ||
.sort((a, b) => b.amcInUnits - a.amcInUnits) | ||
.sort((a, b) => b.stockInUnits - a.stockInUnits); | ||
|
||
const line = lines.find(line => line.id === draft?.id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For fragment to be available to editViewColumns plugin interface |
||
|
||
return ( | ||
<Box display="flex" flexDirection="column" padding={2}> | ||
<Box display="flex" justifyContent="space-between"> | ||
|
@@ -213,6 +218,10 @@ export const RequestLineEdit = ({ | |
label={t('label.amc')} | ||
sx={{ marginBottom: 1 }} | ||
/> | ||
{line && | ||
plugins.requestRequisitionColumn?.editViewColumns?.map( | ||
(Column, index) => <Column key={index} line={line} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should call this |
||
)} | ||
{isProgram && useConsumptionData && ( | ||
<InputWithLabelRow | ||
Input={ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,10 +8,10 @@ use diesel_derive_enum::DbEnum; | |
use serde::{Deserialize, Serialize}; | ||
|
||
#[derive(Clone, Eq, PartialEq, Debug, Serialize, Deserialize)] | ||
#[serde(rename_all = "SCREAMING_SNAKE_CASE")] | ||
#[serde(rename_all = "snake_case")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. snake case now because this maps directly to main plugins type now, made comments in types/mod.rs |
||
pub enum PluginType { | ||
Amc, | ||
TransformRequisitionLines, | ||
AverageMonthlyConsumption, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was not necessary, but not a huge change, sorry for the extra diff |
||
TransformRequestRequisitionLines, | ||
} | ||
|
||
#[derive(Clone, PartialEq, Eq, Debug, Default, Serialize, Deserialize)] | ||
|
@@ -201,7 +201,10 @@ mod test { | |
let repo = BackendPluginRowRepository::new(&connection); | ||
let id = "backend_plugin_row".to_string(); | ||
|
||
let types = PluginTypes(vec![PluginType::Amc, PluginType::Amc]); | ||
let types = PluginTypes(vec![ | ||
PluginType::AverageMonthlyConsumption, | ||
PluginType::AverageMonthlyConsumption, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit, but should we use the TransformRequestRequisitionLines here now? |
||
]); | ||
let _ = repo.upsert_one(BackendPluginRow { | ||
id: id.clone(), | ||
types: types.clone(), | ||
|
@@ -216,6 +219,9 @@ mod test { | |
.unwrap(); | ||
|
||
// Showing that types serializes to a readable text in DB field | ||
assert_eq!(result[0].types, r#"["AMC","AMC"]"#); | ||
assert_eq!( | ||
result[0].types, | ||
r#"["average_monthly_consumption","average_monthly_consumption"]"# | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ use std::ops::Range; | |
|
||
use chrono::{NaiveDate, NaiveDateTime}; | ||
use serde::{Deserialize, Serialize}; | ||
use ts_rs::TS; | ||
use util::inline_init; | ||
|
||
#[derive(Clone, PartialEq, Debug, Default)] | ||
|
@@ -101,16 +102,22 @@ impl StringFilter { | |
} | ||
} | ||
|
||
#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] | ||
#[derive(Clone, PartialEq, Debug, TS, Serialize, Deserialize)] | ||
pub struct EqualFilter<T> | ||
where | ||
T: 'static, | ||
{ | ||
#[ts(optional)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without optional Option is treated as null, with optional treated as undefined (with former have to list all fields ..) |
||
pub equal_to: Option<T>, | ||
#[ts(optional)] | ||
pub not_equal_to: Option<T>, | ||
#[ts(optional)] | ||
pub equal_any: Option<Vec<T>>, | ||
#[ts(optional)] | ||
pub equal_any_or_null: Option<Vec<T>>, | ||
#[ts(optional)] | ||
pub not_equal_all: Option<Vec<T>>, | ||
#[ts(optional)] | ||
pub is_null: Option<bool>, | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ use boa_engine::{ | |
JsValue, Module, Source, | ||
}; | ||
|
||
use repository::PluginType; | ||
use serde::{de::DeserializeOwned, Serialize}; | ||
use thiserror::Error; | ||
|
||
|
@@ -30,16 +31,22 @@ impl PartialEq for BoaJsPluginError { | |
} | ||
} | ||
|
||
fn plugin_type_to_string(r#type: &PluginType) -> String { | ||
serde_json::to_string(r#type).unwrap().replace("\"", "") | ||
} | ||
|
||
pub(crate) fn call_plugin<I, O>( | ||
input: I, | ||
name: &str, | ||
r#type: &PluginType, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this change even though it's a little bit out of scope, it avoids having to hard code string and makes PluginType source of truth |
||
bundle: &Vec<u8>, | ||
) -> Result<O, BoaJsPluginError> | ||
where | ||
I: Serialize, | ||
O: DeserializeOwned, | ||
{ | ||
use BoaJsPluginError as Error; | ||
let r#type = plugin_type_to_string(r#type); | ||
|
||
// Initialise context with loader | ||
let loader = Rc::new(SimpleModuleLoader::new(Path::new("."))?); | ||
let mut context = &mut Context::builder().module_loader(loader.clone()).build()?; | ||
|
@@ -53,28 +60,29 @@ where | |
context.run_jobs(); | ||
match promise.state() { | ||
PromiseState::Fulfilled(JsValue::Undefined) => {} | ||
_ => return Err(Error::LoadingModule(name.to_string())), | ||
_ => return Err(Error::LoadingModule(r#type.clone())), | ||
} | ||
|
||
// TODO should these be bound as camel case ? Also for inputs and outputs ? | ||
methods::log::bind_method(context)?; | ||
methods::sql::bind_method(context)?; | ||
methods::sql_type::bind_method(context)?; | ||
methods::get_store_preferences::bind_method(context)?; | ||
methods::get_plugin_data::bind_method(context)?; | ||
|
||
let namespace = module.namespace(context); | ||
let plugins = namespace | ||
.get(js_string!("plugins"), context)? | ||
.as_object() | ||
.cloned() | ||
.ok_or_else(|| Error::PluginNamespaceMissing(name.to_string()))?; | ||
.ok_or_else(|| Error::PluginNamespaceMissing(r#type.clone()))?; | ||
|
||
let key = js_string!(name); | ||
let key = js_string!(r#type.as_str()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come this one not using |
||
let plugin = plugins | ||
.get(key, context)? | ||
.as_callable() | ||
.cloned() | ||
.ok_or_else(|| Error::PluginMissing(name.to_string()))?; | ||
.ok_or_else(|| Error::PluginMissing(r#type.clone()))?; | ||
|
||
let input: serde_json::Value = serde_json::to_value(&input)?; | ||
let js_input = JsValue::from_json(&input, &mut context)?; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
use boa_engine::*; | ||
use repository::{PluginDataFilter, PluginDataRepository, PluginDataRow}; | ||
|
||
use crate::backend_plugin::{boajs::utils::*, plugin_provider::PluginContext}; | ||
|
||
pub(crate) fn bind_method(context: &mut Context) -> Result<(), JsError> { | ||
context.register_global_callable( | ||
JsString::from("get_plugin_data"), | ||
0, | ||
NativeFunction::from_copy_closure(move |_, args, mut ctx| { | ||
// TODO Is this actually safe ? (need to check reference counts after plugin has run) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would like to know about this before we merge..? |
||
let service_provider = PluginContext::service_provider(); | ||
|
||
let filter: PluginDataFilter = get_serde_argument(&mut ctx, args, 0)?; | ||
|
||
let connection = service_provider | ||
.connection() | ||
.map_err(std_error_to_js_error)?; | ||
|
||
// TODO pagination or restrictions ? | ||
let plugin_data: Vec<PluginDataRow> = PluginDataRepository::new(&connection) | ||
.query_by_filter(filter) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does feel pretty loose, had same thought with |
||
.map_err(std_error_to_js_error)? | ||
.into_iter() | ||
.map(|r| r.plugin_data) | ||
.collect(); | ||
|
||
let value: serde_json::Value = | ||
serde_json::to_value(&plugin_data).map_err(std_error_to_js_error)?; | ||
// We return the moved variable as a `JsValue`. | ||
Ok(JsValue::from_json(&value, ctx)?) | ||
}), | ||
)?; | ||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
pub(crate) mod get_plugin_data; | ||
pub(crate) mod get_store_preferences; | ||
pub(crate) mod log; | ||
pub(crate) mod sql; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
use std::string::FromUtf16Error; | ||
|
||
use boa_engine::{js_string, JsError, JsValue}; | ||
use boa_engine::{js_string, Context, JsError, JsValue}; | ||
use serde::de::DeserializeOwned; | ||
use std::error::Error as StandardError; | ||
use thiserror::Error; | ||
use util::format_error; | ||
|
@@ -19,13 +20,11 @@ pub(super) fn get_string_argument(args: &[JsValue], index: usize) -> Result<Stri | |
use GetStringArgumentError as Error; | ||
|
||
let closure = move || -> Result<String, GetStringArgumentError> { | ||
let Some(arg) = args.get(index) else { | ||
return Err(Error::NoArgumentAtIndex(index)); | ||
}; | ||
let arg = args.get(index).ok_or(Error::NoArgumentAtIndex(index))?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry again refactor, i added new method below which uses ok_or so thought to keep consistent, subjectively no difference in readability Β―_(γ)_/Β― |
||
|
||
let Some(arg) = arg.as_string() else { | ||
return Err(Error::ArgumentAtIndexIsNotAString(index)); | ||
}; | ||
let arg = arg | ||
.as_string() | ||
.ok_or(Error::ArgumentAtIndexIsNotAString(index))?; | ||
|
||
Ok(arg.to_std_string()?) | ||
}; | ||
|
@@ -37,3 +36,31 @@ pub(super) fn std_error_to_js_error(error: impl StandardError) -> JsError { | |
let as_string = JsValue::String(js_string!(format_error(&error))); | ||
JsError::from_opaque(as_string) | ||
} | ||
|
||
#[derive(Debug, Error)] | ||
enum GetJsonArgumentError { | ||
#[error("No argument at index {0}")] | ||
NoArgumentAtIndex(usize), | ||
#[error(transparent)] | ||
JsError(#[from] JsError), | ||
#[error(transparent)] | ||
SerdeError(#[from] serde_json::Error), | ||
} | ||
|
||
pub(super) fn get_serde_argument<D: DeserializeOwned>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is used to get of plugin data filter |
||
context: &mut Context, | ||
args: &[JsValue], | ||
index: usize, | ||
) -> Result<D, JsError> { | ||
use GetJsonArgumentError as Error; | ||
|
||
let mut closure = move || -> Result<D, GetJsonArgumentError> { | ||
let arg = args.get(index).ok_or(Error::NoArgumentAtIndex(index))?; | ||
|
||
let value = arg.to_json(context)?; | ||
|
||
Ok(serde_json::from_value(value)?) | ||
}; | ||
|
||
closure().map_err(std_error_to_js_error) | ||
} |
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.
decided to change this since submodule example in readme uses myPluginBundle as plugin name