ACQ - Updated metadb derived tables #804

arthur-aguilera · 2023-10-23T23:51:27Z

Creates new po_orders derived table and adds the table to the runlist
Updates feesfines_accounts_acounts derived table

syncing arthur-aguilera fork

Updates to feesfines_accounts_actions table based on issue folio-org#501

adds po_notes per Issue folio-org#798

adds po_orders to runlist.

sql_metadb/derived_tables/feesfines_accounts_actions.sql

nassibnassar · 2023-10-24T00:54:57Z

sql_metadb/derived_tables/po_notes

@@ -0,0 +1,31 @@
+--MetaDB: Table po_notes


See the directive syntax at: https://metadb.dev/doc/#_metadbtable

Also note that the name is "Metadb" not "MetaDB".

nassibnassar · 2023-10-24T00:56:36Z

sql_metadb/derived_tables/runlist.txt

@@ -55,6 +55,7 @@ po_lines_eresource.sql
 po_lines_locations.sql
 po_lines_phys_mat_type.sql
 po_lines_physical.sql
+po_orders.sql


This doesn't look right.

Updates po_orders to po_notes to reflect new derived table

adds directive syntax

updates directive syntax

arthur-aguilera · 2023-10-31T15:28:43Z

@nassibnassar Hi Nassib - I updated the queries with the requested changes. I'm not sure how to update the pull request if needed. Thanks!

LauSchl

See code comments

All queries:

Derived tables:

First line is "--metadb:table" directive, followed by blank line
User documentation in comment lines, followed by blank line
File name is listed in runlist.txt after dependencies

sql_metadb/derived_tables/feesfines_accounts_actions.sql

LauSchl · 2023-11-03T10:23:51Z

sql_metadb/derived_tables/feesfines_accounts_actions.sql

+COMMENT ON COLUMN feesfines_accounts_actions.fine_account_id IS 'User fine/fee account id, UUID'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fine_account_amount IS 'Amount of the fine/fee'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fine_date IS 'Date and time the account of the fine/fee was created'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fine_updated_date IS 'Date and time the account of the fine/fee was updated'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fee_fine_id IS 'ID of the fee/fine'
+
+COMMENT ON COLUMN feesfines_accounts_actions.owner_id IS 'ID of the account owner'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fee_fine_owner IS 'Owner of the account'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fee_fine_type IS 'Fee/fine that is up to the desecration of the user'
+
+COMMENT ON COLUMN feesfines_accounts_actions.material_type_id IS 'ID of the material type of the item'
+
+COMMENT ON COLUMN feesfines_accounts_actions.material_type IS 'Material type of the item'
+
+COMMENT ON COLUMN feesfines_accounts_actions.payment_status IS 'Overall status of the payment/waive/transfer/refund/cancel'
+
+COMMENT ON COLUMN feesfines_accounts_actions.fine_status IS 'Overall status of the fee/fine'
+
+COMMENT ON COLUMN feesfines_accounts_actions.account_user_id IS 'ID of the user'
+
+COMMENT ON COLUMN feesfines_accounts_actions.transaction_id IS 'Fine/fee action id, UUID'
+
+COMMENT ON COLUMN feesfines_accounts_actions.transaction_amount IS 'Amount of activity'
+
+COMMENT ON COLUMN feesfines_accounts_actions.account_balance IS 'Calculated amount of remaining balance based on original fee/fine and what has been paid/waived/transferred/refunded'
+
+COMMENT ON COLUMN feesfines_accounts_actions.type_action IS 'Type of activity including the type of transaction'
+
+COMMENT ON COLUMN feesfines_accounts_actions.transaction_date IS 'Date and time the transaction of the fine/fee was created'
+
+COMMENT ON COLUMN feesfines_accounts_actions.transaction_location IS 'The service point where the action was created'
+
+COMMENT ON COLUMN feesfines_accounts_actions.transaction_information IS 'Number or other transaction id related to payment'
+
+COMMENT ON COLUMN feesfines_accounts_actions.operator_id IS 'Person who processed activity (from login information)'
+
+COMMENT ON COLUMN feesfines_accounts_actions.payment_method IS 'Overall status of the action-setting'
+
+COMMENT ON COLUMN feesfines_accounts_actions.user_id IS 'User UUID'
+
+COMMENT ON COLUMN feesfines_accounts_actions.user_patron_group_id IS 'UUID for user patron group'
+
+COMMENT ON COLUMN feesfines_accounts_actions.patron_group_name IS 'User patron group'
+
+COMMENT ON COLUMN feesfines_accounts_actions.signed_transaction_amount IS 'Identifies the type_action value that decreases the balance of the account and adds a - to those values'


All comments are missing a ";" at the end

Added ";" at the end of each comment.

LauSchl · 2023-11-03T10:26:40Z

sql_metadb/derived_tables/feesfines_accounts_actions.sql

+
+COMMENT ON COLUMN feesfines_accounts_actions.transaction_information IS 'Number or other transaction id related to payment'
+
+COMMENT ON COLUMN feesfines_accounts_actions.operator_id IS 'Person who processed activity (from login information)'


wrong column name. Did you mean "action_created_by"?

removed operator_id comment

LauSchl · 2023-11-03T10:28:22Z

sql_metadb/derived_tables/feesfines_accounts_actions.sql

+
+COMMENT ON COLUMN feesfines_accounts_actions.material_type IS 'Material type of the item'
+
+COMMENT ON COLUMN feesfines_accounts_actions.payment_status IS 'Overall status of the payment/waive/transfer/refund/cancel'


wrong column name. Did you mean "payment_status_name"?

changed the name of the column in main query to just "payment_status"

sql_metadb/derived_tables/po_notes

LauSchl · 2023-11-03T10:34:09Z

sql_metadb/derived_tables/po_notes

+
+COMMENT ON COLUMN po_notes.workflow_status IS 'workflow status of the purchase order'
+
+COMMENT ON COLUMN po_notes.po_note IS 'Purchase order note'


wrong column name. Did you mean "note"?

changed to po_notes.note

sql_metadb/derived_tables/po_notes

Added ';' at the end of columns; added back in documentation for the table; changed payment_status_name to payment_status, and removed comment or operator_id

added documentation; added ";" at end of comments; corrected COMMENT ON COLUMN fields to match fields of main query.

arthur-aguilera · 2023-11-09T16:43:03Z

@LauSchl Thank you so much for your comments! I hope I've fixed everything you found and have updated my branch. Thank you!

natalyapik

for po_notes table , it would be nice to be consistent with field names with other derived tables (see po_instance table): instead of "poNumber" we have "po_number", "creation_date" we have "created_date", "po_workflow_status" instead of just "workflow_status", po_note instead of "note"( i think there is also renewalNote and closeReason_note"), po_note_ordinality

natalyapik

also, there is a misspell "--Creates a dervied table" and created date needs to be cast : jsonb_extract_path_text(po.jsonb, 'metadata', 'createdDate')::timestamptz, unless you really need time. Comments need to be adjusted if you rename fields. Thanks, Natalya.

adds renewal and closure notes; updates column names to be aligned with other derived tables

arthur-aguilera · 2023-11-09T18:10:45Z

@natalyapik Thank you! I updated the file with those changes.

natalyapik

Looks great to me! Thanks Arthur.

nassibnassar · 2023-11-10T23:40:44Z

sql_metadb/derived_tables/po_notes

+COMMENT ON COLUMN po_closeReason_note IS 'Notes entered when a purchase order is closed';
+
+COMMENT ON COLUMN po_renewal_note IS 'Notes entered in the ongoing order information';


These lines result in a syntax error because the table name is missing.

nassibnassar · 2023-11-10T23:46:33Z

sql_metadb/derived_tables/feesfines_accounts_actions.sql

+CASE WHEN
+    jsonb_extract_path_text(ff.jsonb, 'typeAction') IN
+    ('Paid partially','Paid fully','Waived partially','Waived fully','Credited partially','Credited fully')
+    THEN jsonb_extract_path_text(ff.jsonb, 'amountAction')::numeric(12,2) * -1
+    ELSE jsonb_extract_path_text(ff.jsonb, 'amountAction')::numeric(12,2)
+    END AS signed_transaction_amount


Maybe this could be indented so that it will not be at the same indentation level as the FROM.

Also there might be some way to avoid the WHEN, THEN, and ELSE all being at the same indentation level.

nassibnassar · 2023-11-10T23:51:10Z

sql_metadb/derived_tables/feesfines_accounts_actions.sql


 DROP TABLE IF EXISTS feesfines_accounts_actions;

 CREATE TABLE feesfines_accounts_actions AS
+


Could we remove blank lines in the middle of a query? Sorry that is not documented anywhere.

nassibnassar · 2023-11-10T23:51:52Z

sql_metadb/derived_tables/po_notes

+DROP TABLE IF EXISTS po_notes;
+
+CREATE TABLE po_notes AS
+


Again if we could remove blank lines in the middle of a query.

nassibnassar · 2023-11-10T23:52:08Z

sql_metadb/derived_tables/po_notes

+po.jsonb -> 'ongoing' ->> 'renewalNote' AS po_renewal_note
+FROM folio_orders.purchase_order AS po
+CROSS JOIN LATERAL jsonb_array_elements(jsonb_extract_path(po.jsonb, 'notes')) WITH ORDINALITY AS po_notes (jsonb);
+;


Is this an extra semicolon?

adds .sql to filename and removes blank lines in middle of query

removes blank lines and changes indentation for case when, then, else, end

arthur-aguilera · 2023-11-13T15:11:51Z

@nassibnassar Hi Nassib, hopefully I've resolved the comments. I made have misunderstood what you were hoping to see with the CASE, WHEN, ELSE, END statement. Thanks!

…s_action.sql Updates indentation for CASE, WHEN, ELSE, END

arthur-aguilera · 2023-11-14T16:21:33Z

@sbeltaine I've updated the indentation for the CASE, WHEN, ELSE, END, per our discussion at ACQ-ERM today.

nassibnassar · 2023-11-14T23:23:47Z

There is so much going on in this PR and I'm having trouble following what was intended. It looks like feesfines_accounts_actions.sql has been accidentally renamed to feesfines_accounts_action.sql. Is that correct?

fixes the name of the file which should be feesfines_accounts_actions.sql

arthur-aguilera · 2023-11-15T01:00:12Z

Sorry @nassibnassar. That shouldn't have happened and that was my error. I returned it back to the correct name. I was going to add ".sql" just in case I missed it like I did with the po_notes, and then realized I had already added it so I went to delete it. I guess I backspaced one character too far. I'm sorry for adding more confusion.

LauSchl · 2023-11-16T14:22:16Z

sql_metadb/derived_tables/po_notes.sql

+COMMENT ON COLUMN po_closeReason_note IS 'Notes entered when a purchase order is closed';
+
+COMMENT ON COLUMN po_renewal_note IS 'Notes entered in the ongoing order information';


The two of these aren't qualified.
I couldn't find any guidelines it it is necessary for this repo but since you added it
for all the other comments it would be consistent to add the tablename here as well.

The table name is needed to avoid a SQL syntax error.

Also could we have po_close_reason_note instead of po_closeReason_note for the column name?

sbeltaine

feesfines_accounts_actions.sql

All queries:

Derived tables:

First line is "--metadb:table" directive, followed by blank line
User documentation in comment lines, followed by blank line
File name is listed in runlist.txt after dependencies

sbeltaine · 2023-11-21T17:26:59Z

sbeltaine · 2023-11-21T17:29:26Z

The po_notes.sql table needs qualified references for tables listed in the Comments.

The feesfines_accounts_actions.sql table needs Comments for the actions_created_by table.

nassibnassar · 2023-11-21T17:31:37Z

@sbeltaine To approve a PR (once it meets your approval in its entirety), please go to: Files changed > Review changes > Approve > Submit review

nassibnassar · 2023-11-23T17:19:53Z

We are getting there. Thanks for your patience.

Could someone explain why two different tables have been combined into a single pull request? It would be easier to keep track of the many comments above if the PR had been simpler.

sbeltaine · 2023-11-28T13:16:38Z

@nassibnassar - Yes, it is better to submit one table in one PR. We reviewed how to do this in our ACQ meeting after walking through this PR, so future PRs will be limited to one table.

Adds missing qualified references for tables listed in the Comments and renames po_closeReason_note to po_close_reason_note per Nassib

Adds comment for feesfines_accounts_actions.action_created_by

arthur-aguilera · 2023-11-28T18:11:32Z

@sbeltaine I think I add those comments and fixed the column name in po_notes. @nassibnassar Yes, the multiple tables on one PR was my bad. Sharon was very helpful for learning how to do this better next time. Thank you!

nassibnassar

Sorry for the many comments, and otherwise it is looking good to me.

nassibnassar · 2023-12-01T16:18:28Z

sql_metadb/derived_tables/po_notes.sql

+po_notes.jsonb #>> '{}' AS po_note,
+po_notes.ORDINALITY AS po_note_ordinality,
+po.jsonb -> 'closeReason' ->> 'note' AS po_close_reason_note,
+po.jsonb -> 'ongoing' ->> 'renewalNote' AS po_renewal_note


I could not find renewalNote in any databases or under "ongoing" in the JSON schema here: https://s3.amazonaws.com/foliodocs/api/mod-orders-storage/p/purchase-order.html

Did I look in the wrong place? Are you sure this is correct?

it is on po_line table: https://s3.amazonaws.com/foliodocs/api/mod-orders-storage/p/po-line.html#orders_storage_po_lines__id__get
and it is extracted in the transformed table.@arthur-aguilera please delete "renewal note" from this derived table.

nassibnassar · 2023-12-01T16:19:05Z

sql_metadb/derived_tables/po_notes.sql

+po.jsonb ->> 'poNumber' AS po_number,
+po.jsonb ->> 'workflowStatus' AS po_workflow_status,
+po_notes.jsonb #>> '{}' AS po_note,
+po_notes.ORDINALITY AS po_note_ordinality,


Please use all lowercase for identifiers. See: https://github.com/folio-org/folio-analytics/blob/main/CONTRIBUTING.md#8-naming-things

nassibnassar · 2023-12-01T16:20:38Z

sql_metadb/derived_tables/po_notes.sql

+(po.jsonb -> 'metadata' ->> 'createdDate')::timestamptz AS created_date,
+po.jsonb ->> 'poNumber' AS po_number,
+po.jsonb ->> 'workflowStatus' AS po_workflow_status,


Please use jsonb_extract_path_text() for JSON extraction (here and below).

nassibnassar · 2023-12-01T16:25:27Z

sql_metadb/derived_tables/feesfines_accounts_actions.sql

 --metadb:table feesfines_accounts_actions	
-
-- Create a derived table that takes feesfines_accounts as the main table
-- join all transaction data from the feesfines_actions table
-- add patron group information from user_group table
+--Creates a derived table that takes folio_feesfines.accounts as the main table


If you would, in both feesfines_accounts_actions.sql and po_notes.sql, please add a blank line after the --metadb:table directive (see https://metadb.dev/doc/#_external_sql_directives) and a blank line in between individual SQL statements below.

nassibnassar · 2023-12-01T16:29:52Z

sql_metadb/derived_tables/po_notes.sql

+SELECT
+po.id AS po_id,


If you would, please consider using indentation to make the query easier to read. For examples, see feesfines_accounts_actions.sql above or the query within: https://github.com/metadb-project/metadb-examples/blob/main/folio/reports/missing_items.sql

nassibnassar · 2023-12-01T16:36:19Z

Creates new po_orders derived table and adds the table to the runlist Updates feesfines_accounts_acounts derived table

Should this say po_notes? In the PR description.

nassibnassar · 2024-07-08T16:06:16Z

This pull request may have unresolved issues.

arthur-aguilera added 4 commits October 23, 2023 17:33

Merge pull request #1 from folio-org/main

3cdcc1c

syncing arthur-aguilera fork

Update feesfines_accounts_actions.sql

4ad8986

Updates to feesfines_accounts_actions table based on issue folio-org#501

Create po_notes

ffd7e7b

adds po_notes per Issue folio-org#798

Update runlist.txt

2e78fc7

adds po_orders to runlist.

arthur-aguilera requested a review from natalyapik October 23, 2023 23:51

arthur-aguilera changed the title ~~Updated metadb derived tables~~ ACQ - Updated metadb derived tables Oct 23, 2023

nassibnassar reviewed Oct 24, 2023

View reviewed changes

sql_metadb/derived_tables/feesfines_accounts_actions.sql Show resolved Hide resolved

nassibnassar reviewed Oct 24, 2023

View reviewed changes

arthur-aguilera added 4 commits October 31, 2023 09:25

Update runlist.txt

bb01785

Updates po_orders to po_notes to reflect new derived table

Update po_notes

e78ea9a

adds directive syntax

Update feesfines_accounts_actions.sql

a423c02

updates directive syntax

Merge branch 'main' into main

fc1667a

nassibnassar added the needs reviewer label Nov 1, 2023

LauSchl self-requested a review November 2, 2023 12:35

LauSchl suggested changes Nov 3, 2023

View reviewed changes

arthur-aguilera added 3 commits November 9, 2023 09:38

Update feesfines_accounts_actions.sql

29a7999

Added ';' at the end of columns; added back in documentation for the table; changed payment_status_name to payment_status, and removed comment or operator_id

Update po_notes

21e3649

added documentation; added ";" at end of comments; corrected COMMENT ON COLUMN fields to match fields of main query.

Merge branch 'main' into main

20c56bc

natalyapik suggested changes Nov 9, 2023

View reviewed changes

Update po_notes

3fcf3de

adds renewal and closure notes; updates column names to be aligned with other derived tables

natalyapik approved these changes Nov 10, 2023

View reviewed changes

nassibnassar removed the needs reviewer label Nov 10, 2023

nassibnassar requested changes Nov 10, 2023

View reviewed changes

arthur-aguilera added 3 commits November 13, 2023 08:06

Update and rename po_notes to po_notes.sql

baec15f

adds .sql to filename and removes blank lines in middle of query

Update feesfines_accounts_actions.sql

4efc90e

removes blank lines and changes indentation for case when, then, else, end

Merge branch 'main' into main

b0c8490

sbeltaine self-requested a review November 14, 2023 16:16

Update and rename feesfines_accounts_actions.sql to feesfines_account…

76189bd

…s_action.sql Updates indentation for CASE, WHEN, ELSE, END

Rename feesfines_accounts_action.sql to feesfines_accounts_actions.sql

f179576

fixes the name of the file which should be feesfines_accounts_actions.sql

LauSchl suggested changes Nov 16, 2023

View reviewed changes

sbeltaine reviewed Nov 21, 2023

View reviewed changes

arthur-aguilera added 3 commits November 28, 2023 11:06

Update po_notes.sql

88cff5c

Adds missing qualified references for tables listed in the Comments and renames po_closeReason_note to po_close_reason_note per Nassib

Update feesfines_accounts_actions.sql

afe3e1b

Adds comment for feesfines_accounts_actions.action_created_by

Merge branch 'main' into main

38ff0a2

nassibnassar requested changes Dec 1, 2023

View reviewed changes

nassibnassar added the on hold label Jul 8, 2024


		COMMENT ON COLUMN feesfines_accounts_actions.transaction_information IS 'Number or other transaction id related to payment'

		COMMENT ON COLUMN feesfines_accounts_actions.operator_id IS 'Person who processed activity (from login information)'


		COMMENT ON COLUMN po_notes.workflow_status IS 'workflow status of the purchase order'

		COMMENT ON COLUMN po_notes.po_note IS 'Purchase order note'

		COMMENT ON COLUMN po_closeReason_note IS 'Notes entered when a purchase order is closed';

		COMMENT ON COLUMN po_renewal_note IS 'Notes entered in the ongoing order information';


		DROP TABLE IF EXISTS feesfines_accounts_actions;

		CREATE TABLE feesfines_accounts_actions AS

		SELECT
		po.id AS po_id,

ACQ - Updated metadb derived tables #804

Are you sure you want to change the base?

ACQ - Updated metadb derived tables #804

Conversation

arthur-aguilera commented Oct 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthur-aguilera commented Oct 31, 2023

LauSchl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthur-aguilera commented Nov 9, 2023

natalyapik left a comment

Choose a reason for hiding this comment

natalyapik left a comment

Choose a reason for hiding this comment

arthur-aguilera commented Nov 9, 2023

natalyapik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthur-aguilera commented Nov 13, 2023

arthur-aguilera commented Nov 14, 2023

nassibnassar commented Nov 14, 2023

arthur-aguilera commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbeltaine left a comment • edited Loading

Choose a reason for hiding this comment

sbeltaine commented Nov 21, 2023 • edited Loading

sbeltaine commented Nov 21, 2023 • edited Loading

nassibnassar commented Nov 21, 2023 • edited Loading

nassibnassar commented Nov 23, 2023

sbeltaine commented Nov 28, 2023

arthur-aguilera commented Nov 28, 2023

nassibnassar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natalyapik Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nassibnassar commented Dec 1, 2023

nassibnassar commented Jul 8, 2024

sbeltaine left a comment •

edited

Loading

sbeltaine commented Nov 21, 2023 •

edited

Loading

sbeltaine commented Nov 21, 2023 •

edited

Loading

nassibnassar commented Nov 21, 2023 •

edited

Loading

natalyapik Dec 4, 2023 •

edited

Loading