Dashboard

OLMIS-4142: test StockCard

    • -0
    • +75
    /src/stock-card/stock-card.spec.js
Change label from "Remove Requisition Offline" to "Remove Offline Requisition"

    • -1
    • +1
    /src/requisition-search/messages_en.json
Got it, makes sense. You could expose this to Coldtrace I think and you'd get rid of the third state. In fact even with it as is, Coldtrace could be exposed to it with these steps: 1. Coldtrace PU...

Got it, makes sense. You could expose this to Coldtrace I think and you'd get rid of the third state. In fact even with it as is, Coldtrace could be exposed to it with these steps:

1. Coldtrace PUT's a new alert
2. A user dismisses the alert (dismissed == true)
3. Coldtrace PUT's to the same alert with the endTimestamp set.

In step 3, in the response to the PUT, Coldtrace will see dismissed == true as it's no longer null.

It's fine either way I believe, but I do think you could eliminate a potential source of NPE without confusing Coldtrace.

Correct.

Correct.

To determine if there is an alert, or not an alert, I'd presume that there is a query paramater like "activeAlerts" (means open and not dismissed here). If the collection return is empty for that d...

To determine if there is an alert, or not an alert, I'd presume that there is a query paramater like "activeAlerts" (means open and not dismissed here). If the collection return is empty for that device, than there's no alert. If it has > 0 items in the collection then there's an alert. No?

My overall point is that it's good restful practice to return the full set without query parameters, but I'd presume the most common use-case is to get alerts that are "active"*. Who cares if theres an alert that's not useful anymore from 5 years ago?

  • active/inactive - I'm not sure this is the right term to use for open and not-dismissed. Just needed a word to use here.
Fair enough.

Fair enough.

I think what you're saying is: null is a valid state, when coldtrace PUTs it won't be there (null). What's returned as a response to that PUT it also won't be there (null), etc. Correct?

I think what you're saying is: null is a valid state, when coldtrace PUTs it won't be there (null). What's returned as a response to that PUT it also won't be there (null), etc. Correct?

I'm not sure if that's true, as the other tickets which are going to be using this API need to get both active and inactive alerts, to determine which type of alert icon to show, and then also pote...

I'm not sure if that's true, as the other tickets which are going to be using this API need to get both active and inactive alerts, to determine which type of alert icon to show, and then also potentially to show historical inactive alerts. I haven't seen the need to only retrieve the active ones so far.

Not sure; this seems to be the pattern that is done in this service.

Not sure; this seems to be the pattern that is done in this service.

I'm wondering once we need to be able to dismiss alerts, I was thinking we might use the same API to update an alert to dismiss it. Unless you think it would be a separate API to do so?

I'm wondering once we need to be able to dismiss alerts, I was thinking we might use the same API to update an alert to dismiss it. Unless you think it would be a separate API to do so?

Nikodem Graczewski: I can see how "performance" of the page would be out of scope for OLMIS-685 which is about a specific piece of functionality on it. On the other hand we wouldn't want to releas...

Nikodem Graczewski: I can see how "performance" of the page would be out of scope for OLMIS-685 which is about a specific piece of functionality on it.

On the other hand we wouldn't want to release a PoD page that didn't react well when receiving 100s or 1000s of items.

In terms of process for the group, i.e. adding a new ticket or not. I think you can feel free to add it, however I wouldn't want it be in our grooming - we wouldn't at that time truly appreciate what this was or the LOE. I think if you could just get this done, through a task ticket that you add to your sprint in this epic, or you just get it done as part of a related ticket that'll be done in 3.3 I think is up to you. But we certainly should get it done and showcased properly. It sounds like Nick Reid has some good suggestions for how to do it that we have standardized.

Let us know what you'd like to do to get it performing well and how to put it into a sprint before 3.3 ships. If you need any help let me know. Thanks!

Looking good. I left a few comments inline. Thanks!

Looking good. I left a few comments inline. Thanks!

I feel as if the most common operation for us here is to by default only get the alerts from the database that are: *open (as in there's no end time) *not dismissed Is that parameter to limit w...

I feel as if the most common operation for us here is to by default only get the alerts from the database that are:

  • open (as in there's no end time)
  • not dismissed


Is that parameter to limit what the query does coming next?

The problem I see here is that if a message is sent from Coldtrace to OpenLMIS, and it fails validation, then Coldtrace will get some error message, but we may not have much in our logs. It'd be su...

The problem I see here is that if a message is sent from Coldtrace to OpenLMIS, and it fails validation, then Coldtrace will get some error message, but we may not have much in our logs. It'd be super helpful if we ensured we had a good log of what doesn't pass our validation of their message.

OLMIS-4077: Update delete-related tests to use confirmDestroy

Just so I know, are we moving away from Spring Validator?

Just so I know, are we moving away from Spring Validator?

+1 slick

+1 slick

I'm not sure we should do this. Do we have any business logic around the type of alert it is? My assumption is that Coldtrace cares, and we care that they sent us an alert of a certain type. But ot...

I'm not sure we should do this. Do we have any business logic around the type of alert it is? My assumption is that Coldtrace cares, and we care that they sent us an alert of a certain type. But otherwise we'll only ever show what Coldtrace sent us. By putting this as an enum, we couple our understanding of the types of alerts that Coldtrace can generate to our code. i.e. if they add a new alert type, won't our software have to updated as well to understand it?

Minor, but you may want to make this boolean and avoid the possibility of a NPE. I know it won't come from Coldtrace, however that concern can be handled in the constructor where you default this t...

Minor, but you may want to make this boolean and avoid the possibility of a NPE. I know it won't come from Coldtrace, however that concern can be handled in the constructor where you default this to false.

it's better to have bulder in same package as class so we can use method with package private scope

it's better to have bulder in same package as class so we can use method with package private scope

OLMIS-4114: fluent builder for soh calculated

OLMIS-4114: add more tests for requisition update validators
OLMIS-4114: add more tests for requisition update validators
OLMIS-4114: add more tests for requisition update validators

OLMIS-4101: Merged full-supply and non-full-supply controllers

OLMIS-4142: test StockCard
OLMIS-4142: test StockCard
OLMIS-4142: use always string on controller

OLMIS-4101: Moved delete line item logic to Requisition class
OLMIS-4101: Moved delete line item logic to Requisition class
OLMIS-4142: Fix stock adjustments do not allow adding more products
OLMIS-4142: Fix stock adjustments do not allow adding more products