Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

N°7385 - Trigger on mention executed even if it's not a mention with @#638

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
accognet merged 5 commits intodevelopfromfeature/7385-on_mention
Apr 18, 2025

Conversation

accognet
Copy link
Contributor

Internal

@accognetaccognet added the bugSomething isn't working labelMar 22, 2024
@accognetaccognet self-assigned thisMar 22, 2024
Copy link
Contributor

@MolkobainMolkobain left a comment

Choose a reason for hiding this comment

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

Review of the current proposition
Mentions are not only for Contact (@ marker by default), it can be for any kind of objects. So your current implementation breaks it for all other classes.

If we are to go further with this ticket, then we must check if the mentioned object matches the marker it begins with.

Use case that led to this PR
When pasting text from an iTop; if the text contains the hyperlink to an object with the same meta data as the mentions, then the mention will be triggered again even if the hyperlink doesn't start with the marker (@ for Contact).

@v-dumas had this behaviour recently and thought it was nice. So there is obviously a discussion to be done, so we all decide what behaviour we want.

@Molkobain
Copy link
Contributor

I'm moving this PR to "Functional review" even though the technical one didn't occur yet, so the important point can be discussed before going any further.

@Molkobain
Copy link
Contributor

Functional review:

  • Expected behavior: Only trigger a mention, if the hyperlink starts with the corresponding marker (e.g.@ forPerson)
  • Target version: 3.2.x if code changes are not too risky, 3.3.0 otherwise. Will be determined, once PR is corrected

@jf-cbdjf-cbd added the internalWork made by Combodo labelDec 19, 2024
@rquetiez
Copy link
Contributor

rquetiez commentedJan 28, 2025
edited
Loading

Code review : the preceding character must be dehardcoded from '@' to the one given in the configuration, depending on the object class. Also consider a multi-byte safe function.

@accognetaccognetforce-pushed thefeature/7385-on_mention branch from7334dcc to43c1633CompareJanuary 30, 2025 08:19
Copy link
Contributor

@rquetiezrquetiez left a comment

Choose a reason for hiding this comment

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

  1. Two little typographic rules to fix
  2. Having utilsTest derived from iTopDataTestCase seems overkill Is there an alternative?
  3. My mistake on my previous review: multi-byte safe function has no sense when it comes to checking the very first character of a byte sequence. Moreover, a test has been added with an emoji as the "marker".

Copy link
Contributor

@rquetiezrquetiez left a comment

Choose a reason for hiding this comment

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

Ok for merging

@accognetaccognetforce-pushed thefeature/7385-on_mention branch from9a726c6 toc8fe294CompareApril 18, 2025 12:51
@accognetaccognet changed the base branch fromsupport/3.2 todevelopApril 18, 2025 12:51
@accognetaccognet merged commitb21a02d intodevelopApr 18, 2025
@accognetaccognet deleted the feature/7385-on_mention branchApril 18, 2025 12:52
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@MolkobainMolkobainMolkobain requested changes

@rquetiezrquetiezrquetiez approved these changes

Assignees

@accognetaccognet

Labels
bugSomething isn't workinginternalWork made by Combodo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@accognet@Molkobain@rquetiez@jf-cbd

[8]ページ先頭

©2009-2025 Movatter.jp