-
Notifications
You must be signed in to change notification settings - Fork 12
Qualification default expiration time #1630
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
base: main
Are you sure you want to change the base?
Qualification default expiration time #1630
Conversation
…itile as possible
felixrindt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time coming up with a scheme to store default expiration durations. I do like the flexibility you built into the scheme.
- If we introduce a default expiration to the model it should be part of the Qualification form in a way that non technical people can easily understand how it works. I don't think that's the case here. We could provide some multipart widget that offers selection of different modes.
- With that in mind, I wish the model can be simpler than what you suggest. What would anyone need a fixed year or relative days for? I can probably think of something, but what were your thoughts?
- We should also provide a function that applies this default duration to a start date. That shouldn't be part of your plugin. But what would the start date actually be when adding a qualification to a user? The time of editing the QualificationGrant? Should we also save the date from which the grant is valid?
- model-wise, I'm not happy with having this complex datatype as a plain charfield with validator. Maybe we can encapsulate serialization, validation and functionality in a class with it's own Model and Form field. That would produce code like
expiration: RelativeTime = qualification.default_expiration_relative_time # using a RelativeTimeModelField
grant.valid_to = expiration.apply_to(grant.valid_from)|
Hi @samuelson-ben, we had another discussion about your proposal.
We decided that we want to offer a multipart widget for the user. This can be a very simple version in the first iteration. We thought about radio buttons distinguishing between "after x years" and "at x.y. after z years". This should cover the common usecases and can still be extended later. The data should be stored in a JSONField in the database in the form of
For convenience, this should be offered by a custom formfield which we can also reuse in other situations (e.g. #209). Would you feel comfortable implementing that or should we support you in that regard? |
|
@jeriox @felixrindt sorry for not responding in the past. I found some time and threw lines of code against my vm and got something like: Currently I am storing the day month and year as integers in json (I hope). I will fix some wrong texts and clean up some messes in my code (Sorry for every bit of bad quality you have to see in the code but this little integration of something is the biggest thing I have every done in Python). I hope I will get to it tomorrow and then I can present it to you. Also I have to implement a way for giving a qualification date and calculation based on these values the expiration date. |
|
I cleaned up a bit, provided translations and added the new json format to the qualification-fixtures. |
|
@felixrindt @jeriox |
|
Hi @felixrindt and @jeriox |
|
Hi @samuelson-ben, sorry for not replying for such a long time, we were both swamped with work. I just took the time to look at your PR. A few comments:
I will prepare a PR against your branch with those suggestions. Thanks again for your work so far! |
|
@samuelson-ben my PR is ready, feel free to check it out and merge it into your branch. Happy to hear your thoughts on it |
simplify expiration handling
…rite Revert "simplify expiration handling" due to some simple errors

Hi,
here a pr for the topic of the qualification requests.
As part of it, it would be suitable to have a default expiration time in the core so not every plugin as to build it itself.
See also ephios-dev/ephios-qualification-fixtures#7 as the changes for the import repo.
Please look over it and give some feedback.
I know the way I solved the dilemma between having a fixed time and a set at the end of year isn't great but I don't want to introduce many new fields and it should be able to set something like the 15. May of the following year and end of year or month wouldn't be suitable.
I am open for other ideas.