Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add (and refactor) expiration helpers #1281

Closed
wants to merge 14 commits into from

Conversation

PolariTOON
Copy link
Contributor

@PolariTOON PolariTOON commented Dec 5, 2022

This PR is an alternative to #1274, that exports the same API, with an equivalent behavior, but with a final refactor to shape it in a more declarative way, inspired by #1274 (comment) .
Thus, it can also be seen as a follow-up refactor that can land afterwards.
Therefore, the main difference is in ee5d836 and the next commits.

This avoids using `paperDefinitions` as a config.
It may not be generic enough in the future and
we don't have any need for that right now.
`isExpiring` can be used instead.
This removes the need to pass a `dateLabel` argument to these helpers.
It will be used in `cozy-ui`.
They will be used in `cozy-ui`, `cozy-mespapiers-lib` and `mespapiers`.
This removes the need for `lodash/get` in `paper.js`
Paper won't be considered as expiring if the user
has not set any notice period.
Thus the alert won't appear and no notification will be sent.
@PolariTOON PolariTOON force-pushed the refactor/refactor-expiration-helpers branch from 0cc7ed9 to 1aa29b9 Compare December 5, 2022 16:38
@PolariTOON
Copy link
Contributor Author

This PR was opened to provide an alternative implementation to #1274. It is currently not clear if it's better and it makes noise, so for now let's close this PR to focus on #1274 instead because that's where most of the discussion happened. If later on it appears this way of defining each paper on a case by case basis is better, we still be able to revive it on top of the first one.

@PolariTOON PolariTOON closed this Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant