Sebastian Brudziński

Modified thresholds for performance tests

* FTAPs and Orderables are more restrictive now

* All Supply Lines with Expand is a little less restrictive

    • -1
    • +1
    /performance/tests/approvedProductsForEssentialMedsAndDistrictHospital.yml
Why do we need a method that only calls another method? https://review.openlmis.org/static/ogdo0b/2static/images/wiki/icons/emoticons/smile.gif

Why do we need a method that only calls another method?

To improve readability we could: 1. merge this with an already existing if-statement 2. Extract this check to a separate private method

To improve readability we could:
1. merge this with an already existing if-statement
2. Extract this check to a separate private method

Why do we need this step if we are returning null anyways?

Why do we need this step if we are returning null anyways?

Is that still necessary?

Is that still necessary?

We should be using constants for all of the literals

We should be using constants for all of the literals

Can't we use the constant from the abstract class?

Can't we use the constant from the abstract class?

We should extract all of those literals as constants

We should extract all of those literals as constants

Also, +1 for splitting the common code to a separate class. Good job!

Also, +1 for splitting the common code to a separate class. Good job!

Instead of this essay can just say "Active flag of ProgramOrderables does no longer impact FTAP endpoints"

Instead of this essay can just say "Active flag of ProgramOrderables does no longer impact FTAP endpoints"

Don't mention requisitions in referencedata

Don't mention requisitions in referencedata

Drop "or not"

Drop "or not"

Can't we request a specific version?

Can't we request a specific version?

drop "or not"

drop "or not"

Why keep it here if we have that in the abstract class already?

Why keep it here if we have that in the abstract class already?

Let's not call this a util class if it's abstract. I'd go with IdentitiesSearchableRepository perhaps?

Let's not call this a util class if it's abstract. I'd go with IdentitiesSearchableRepository perhaps?

Doesn't criteria builder offer equals with ignore case? That would make the code a little easier to read

Doesn't criteria builder offer equals with ignore case? That would make the code a little easier to read

That is not correct. Last-Modified and If-Modified-Since headers should always use GMT: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Last-Modified https://developer.mozilla.org/en-US/d...
This is the date that we use in Last-Modified and If-Modified-Since, right? If so, it should use GMT: "HTTP dates are always expressed in GMT, never in local time." - https://developer.mozilla.org/...

This is the date that we use in Last-Modified and If-Modified-Since, right? If so, it should use GMT:
"HTTP dates are always expressed in GMT, never in local time." - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Last-Modified https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since

This is the date that we use in Last-Modified and If-Modified-Since, right? If so, it should use GMT: "HTTP dates are always expressed in GMT, never in local time." - https://developer.mozilla.org/...

This is the date that we use in Last-Modified and If-Modified-Since, right? If so, it should use GMT:
"HTTP dates are always expressed in GMT, never in local time." - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Last-Modified https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since

In case of single field selection this is a smaller issue, but if possible yes - let's avoid native query

In case of single field selection this is a smaller issue, but if possible yes - let's avoid native query

Swallowing exceptions is an anti-pattern. At the very least, we should log them. In this case, it seems that we don't need to catch those exceptions at all - why are we doing this?

Swallowing exceptions is an anti-pattern. At the very least, we should log them. In this case, it seems that we don't need to catch those exceptions at all - why are we doing this?

100 or 1k?

100 or 1k?

900 or 9k?

900 or 9k?

1k or 10k?

1k or 10k?

This will generate 9k UUIDs that do not exist in the database and ultimately we won't retrieve anything

This will generate 9k UUIDs that do not exist in the database and ultimately we won't retrieve anything