Concurrency issues - new constraints

[Sebastian Brudziński]

Hello everyone,

as an outcome of the last technical committee call, we have taken a closer look at the missing constraints for some of the entities that have started seeing duplicate entries. Please take a look and let us know whether the core team feels comfortable with them, in which case we will begin the implementation.

Thanks!
Sebastian.

status_changes
Here's the current definition of the table
    Column     |           Type           | Modifiers |
---------------+--------------------------+-----------+
 id            | uuid                     | not null  |
 createddate   | timestamp with time zone |           |
 modifieddate  | timestamp with time zone |           |
 authorid      | uuid                     |           |
 status        | character varying(255)   | not null  |
 requisitionid | uuid                     | not null  |

A simple unique constraint won't do here as each requisition can reach most of the states multiple times.
We have also considered checking previous status change, but this won't work for requisitions that require multiple approvals (they are not reaching a new state with each approval). Moreover, this would most likely involve writing a trigger as a check constraint can only operate on a single row.
Another identified option was using UI timestamps, but this wouldn't help if users refreshed their page and sent a new request. This also wouldn't prevent duplicates in case of double click or loading modal fading away (unless timestamp was generated at the page load instead of on action button click).
One of the solutions that I can see would be to include an ID of the previous status change and enforce that a new status change is not preceeded by the same one twice. As a drawback, this would require quite complicated migration.
Does anyone see a better constraint here that would ensure uniqueness of a status change?


status_messages
     Column      |           Type           | Modifiers
-----------------+--------------------------+-----------
 id              | uuid                     | not null
 createddate     | timestamp with time zone |
 modifieddate    | timestamp with time zone |
 authorfirstname | character varying(255)   |
 authorid        | uuid                     |
 authorlastname  | character varying(255)   |
 body            | character varying(255)   | not null
 status          | character varying(255)   | not null
 requisitionid   | uuid                     | not null

The simplest solution would be to associate the comment (status_message) with a status_change. You can only add one comment per single status change and therefore unique constraint on status_change ID would be sufficient, once we ensure that status_changes are unique. This would encounter similar problems as the entity above with timestamp UI or status transition.


stock_adjustments
        Column         |  Type   | Modifiers
-----------------------+---------+-----------
 id                    | uuid    | not null
 quantity              | integer | not null
 reasonid              | uuid    | not null
 requisitionlineitemid | uuid    |

There appears to be a simple constraint that can be added - reasonid + requisitionlineitemid combination should be unique, which means you can add one stock adjustment type once for a line item.
The question remains how to handle that on UI - it currently allows adding multiple adjustments of the same type.
I'd suggest removing the adjustment type from the dropdown once we have added it for a line item. Alternatively the UI could add up quantities for the same adjustment types and send an aggregate to the API. Thoughts?

--

Sebastian Brudziński
Software Developer / Team Leader
sbrud...@soldevelo.com

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

[Sebastian Brudziński]

Any thoughts on this, anyone?

Regards,
Sebastian.

[Paweł Gesek]

I think only the first one is 'controversial'. The logic you propose sounds right to me - have you considered using a Check constraint for making this check?

Regards,

Paweł

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/20fa8a6b-d459-d8b3-a771-5917eb49550b%40soldevelo.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875

[Sebastian Brudziński]

If we were to implement the constraint that I'm proposing, a simple unique constraint will be enough since we just need to ensure that we never reference the same previous status change ID twice (so unique on previous status change id). For the other ideas that were raised during tech call - I've explained why the check constraint is not an option in the original post (check constraints can only operate on a single row - the one we are adding). 

Best regards,
Sebastian.

[Paweł Gesek]

Sorry, missed the line about the check constraint. In that case it's probably best to think about storing additional data as you suggest and doing a migration, rather than a trigger.

Thinking about migration - I believe this is something we would be able to do with it sql only. We should be able to figure out the previous status update row for a requisition right?

Regards,
Paweł

[Sebastian Brudziński]

Yes, based on the created date timestamp. We may however need to handle a case where two timestamps are the same.

Regards,
Sebastian.