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

Create default forms on installation or update #17942

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

AdrienClairembault
Copy link
Contributor

Checklist before requesting a review

Please delete options that are not relevant.

  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.

Description

Add two default forms when installing GLPI or updating to GLPI 11.
The goal is to copy the "old" helpdesk form:

image

It is split into two Incident/Request forms which are almost identical.
Note that the "associated elements" question is missing because it isn't possible to implement it right now (we will work on that later) and that the picked "watchers" are also not applied into the ticket (not possible yet too).

The default forms themselves are subject to change later as we make the final adjustments before GLPI 11.
Right now, the goal is just to offer the same feature that in GLPI 10 but with two "real" forms.

@AdrienClairembault AdrienClairembault self-assigned this Sep 26, 2024
@AdrienClairembault AdrienClairembault force-pushed the feature/create-default-forms branch 4 times, most recently from 38a15c3 to f2e0cf7 Compare September 26, 2024 13:57
@AdrienClairembault AdrienClairembault marked this pull request as ready for review September 26, 2024 13:58
@AdrienClairembault
Copy link
Contributor Author

Update tests fail with MariaDB, probably because it doesn't compare json correctly (as JSON is only a real type in MySQL).
We probably need to use the dedicated json_equals somewhere if we are on MariaDB but I am not sure if we want to waste time working on that (for a false positive).

image

Suggestions are welcome.

@AdrienClairembault
Copy link
Contributor Author

Nvm it is actually MySQL that fails the comparison 🤷
I'm tempted to just ignore these two json fields in the update tests, ok for you @cedric-anne ?

@trasher
Copy link
Contributor

trasher commented Sep 27, 2024

I'm tempted to just ignore these two json fields in the update tests

I guess the problem is the tests suite is not ready for that kind of data; and therefore should be fixed; not ignored. Anyway, I do not know well that part, I've no proposition so far.

@AdrienClairembault
Copy link
Contributor Author

I guess the problem is the tests suite is not ready for that kind of data; and therefore should be fixed; not ignored. Anyway, I do not know well that part, I've no proposition so far.

I agree but we have to pick our priorities at this time and this specific false positive case is very low in my TODO list.
If anyone else has more free time, feel free to correct it.

@cedric-anne cedric-anne added this to the 11.0.0 milestone Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants