don't display archived/trashed entities #1358
Conversation
There was a problem hiding this comment.
How will you account for when wanting to filter trashed/archived entities by their status (for example a report that filters all tickets that were sold out including archived tickets)? I grok having the default not include trashed/archive but I think there still should be a way to override that in the call.
Also, I see there are no unit tests for this helper. Can you add some please?
| export const isActive = ( DateTimeEntity, includeTrashed = false ) => { | ||
| return ( | ||
| ( includeTrashed && assertDateTimeEntity( DateTimeEntity ) ) || | ||
| ( ! includeTrashed && ! isTrashed( DateTimeEntity ) ) |
There was a problem hiding this comment.
These conditionals are repeated multiple times. Would there be value in extracting it into it's own callback to keep things DRY? Something like (naming maybe could be better):
const hasValidDateAndIncludesTrashCondition = ( DateTimeEntity, includeTrashed ) => {
return ( includeTrashed && assertDateTimeEntity( DateTimeEntity ) ) ||
( ! includeTrashed && ! isTrashed( DateTimeEntity ) );
}There was a problem hiding this comment.
oooo good point... dunno how I didn't notice that! will do
| } ); | ||
| describe( 'isActive()', () => { | ||
| it( 'returns false if Date Entity start and end dates are in future', | ||
| () => expect( isActive( upcomingDateEntity() ) ).toBe( false ) |
There was a problem hiding this comment.
This returns the value of the expect call. Test callbacks should not return values (see jest-community/eslint-plugin-jest#136).
| if ( | ||
| ( includeArchived && ! assertTicketEntity( TicketEntity ) ) || | ||
| ( ! includeArchived && isArchived( TicketEntity ) ) | ||
| ) { |
There was a problem hiding this comment.
I have the same feedback here as the status-helper for date-times regarding extracting out this common condition into it's own function that you can call here and elsewhere (to keep code DRY).
tn3rb
force-pushed
the
EDTR/archived-entity-status
branch
from
3662bd2
to
89e1909
Compare
Problem this Pull Request solves
Archived/deleted entities still appear in the entity lists when using normal filters but this should not be the case.
This simply prevents trashed entities from being considered when checking the status for an entity.
plz note:
I removed several calls to
assertDateTimeEntity()andassertTicketEntity()because that check is performed byisTrashed()andisArchived()so the check would be redundant.How has this been tested