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

Allow specific display preferences to be configured for the helpdesk #18001

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AdrienClairembault
Copy link
Contributor

@AdrienClairembault AdrienClairembault commented Oct 3, 2024

Checklist before requesting a review

  • 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

  • Allow specific display preferences to be configured for the helpdesk interface.
  • This view is only added for itemtypes that define a supportHelpdeskDisplayPreferences() method that return true (so only ticket for now).
  • There are no restrictions on the criteria that can be selected, despite some ticket's search criteria being disabled for the helpdesk interface (see Ticket::rawSearchOption()).
    This is because:
    • It is hard to filter the selectable criteria depending on the interface because of bad session management in the code.
      We would need a proper service that accept a given interface and then return the available search options.
      Instead, we have a direct call to Session::getCurrentInterface() deeply nested into the stack trace, which mean passing a specific interface value would require adding new parameters to every method in the call stack until we reach Ticket::rawSearchOption().
    • Ticket::rawSearchOption() do not actually remove search options for the helpdesk, it set them as nosearch = true.
      This mean they can't be used to search data but are actually valid to be used as display preferences.
      Thus, it is the administrator responsibility to not add columns that may contain sensitive informations in the helpdesk view.

Screenshots

image

@AdrienClairembault AdrienClairembault self-assigned this Oct 3, 2024
@AdrienClairembault AdrienClairembault force-pushed the feature/helpdesk-view branch 4 times, most recently from d563afb to e168fd0 Compare October 4, 2024 10:24
}
}

public function buildUpdateOrInsert($table, $params, $where, $onlyone = true): string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up not needing this new method, I've still kept it as it may be useful to someone else one day.

@AdrienClairembault
Copy link
Contributor Author

@cedric-anne I'm stuck on this one.

This PR add a new column to the display_preference unicity constraint.
This result in the key being too long for MyISAM (works perfectly on innoDB).

Can we discuss it on monday ?

image

It works on MariaDB tho, I thought it was because of this:

image

So I added USING HASH to the query to try to get the same "limitless" length but without success.

@cconard96
Copy link
Contributor

@cedric-anne I'm stuck on this one.

This PR add a new column to the display_preference unicity constraint. This result in the key being too long for MyISAM (works perfectly on innoDB).

Seems unlikely we will have more than helpdesk or central interfaces. At least for now, this could be a is_helpdesk or is_central tinyint column which should get it under the key length limit.

@trasher
Copy link
Contributor

trasher commented Oct 4, 2024

Maybe interface can be an tinyint with just constants in code GLPI side?

@glpi-project glpi-project deleted a comment from sonarcloud bot Oct 18, 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