-
-
Notifications
You must be signed in to change notification settings - Fork 359
[17.0][IMP] helpdesk_type: Portal ticket type #725
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
Conversation
Hi @max3903, @nelsonramirezs, |
2e59a7a
to
c37f97a
Compare
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.
Translations are missing, but otherwise looks good to me
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.
If you need to, add an initial commit to helpdesk_mgmt
to add identifiers/name to the portal elements you need to make selectors easier and more useful in the future.
c37f97a
to
dc9066d
Compare
ping @victoralmau @david-banon-tecnativa some changes |
dc9066d
to
8e25ef6
Compare
Portal user can't create ticket when show type in portal is enabled because it has no permissions to view types, missed that on my last review |
8e25ef6
to
1a79ac6
Compare
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.
Looks good to me
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.
Functional review OK.
IMO I don't like defining the options this way
I think it would be more appropriate to create a "section" for each field (teams, types and categories).
What do you think @pedrobaeza ?
Well, if everything is related a the "new ticket" form in portal, it's OK to go in the same config section, although being one for teams and another for types, but texts should be homogenized and texts rationalized for UX. |
like this? @pedrobaeza @victoralmau |
1a79ac6
to
ce224c7
Compare
ping @pedrobaeza review and merge please? |
/ocabot merge nobump |
/ocabot merge minor |
Hey, thanks for contributing! Proceeding to merge this for you. |
What a great day to merge this nice PR. Let's do it! |
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 17.0-ocabot-merge-pr-725-by-pedrobaeza-bump-nobump. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
Congratulations, your PR was merged at 0c9860c. Thanks a lot for contributing to OCA. ❤️ |
@Tecnativa TT56029
@victoralmau @pedrobaeza @david-banon-tecnativa