feat: support mention in Research descriptions, updates and comments

[thisislawatts]

PR Checklist

• - Commit messages are descriptive, it will be used in our Release Notes

PR Type

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Developer experience (improves developer workflows for contributing to the project)

Description

What this PR does

Git Issues

Closes #

Screenshots/Videos

If useful, provide screenshot or capture to highlight main changes

---

What happens next?

Thanks for the contribution! We try to make sure all PRs are reviewed ahead of a monthly dev call (first Monday of the month, open to all!).

If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.

If you need more immediate feedback you can try reaching out on Discord in the Community Platform development channel.

[cypress]

Passing run #2920 ↗︎

0

53

0

0

0

Details:

Merge branch 'master' into feat/research-mention-notifications
Project: onearmy-community-platform	Commit: fed2800108
Status: Passed	Duration: 02:54 💡
Started: Feb 16, 2023 7:28 AM	Ended: Feb 16, 2023 7:31 AM

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[github-actions]

Visit the preview URL for this PR (updated for commit fed2800):

https://onearmy-next--pr2072-feat-research-mentio-rtlzpbzd.web.app

_{(expires Mon, 20 Mar 2023 13:54:16 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 6d65e4f8fee2f6ab2da0c1c3b85b8797d66afa59}

[davehakkens]

Thanks @thisislawatts nice feature!
All seems to work here, mentions are linked + notifications triggered.
Only thing is that the text of the notification is empty.
The last four blanc notifications are

1. Mentioned in first post
2. Mentioned in a step
3. Mentioned in research comment
4. Mentioned in research comment
5. (Does work for how-to)

[thisislawatts]

@davehakkens what should the wording be for these notifications. Do we want to specify whether a user was mentioned in a Research description, Update or comment?

[davehakkens]

We could make it very basic and say for all mentions (content + comment) the same:
[username] mentioned you here
Direct to wherever the mention is made.

However you already made some better/ more advanced behaviour in the howto mentions. Separating a comment from the content.

• Mentioned in a how-to
• Mentioned in a comment

So I would continue like this and do the same for research

• Mentioned in a research (description + update)
• Mentioned in a comment

[thisislawatts]

When testing this manually with a URL like: http://localhost:3000/research/testing-mentions#update-0-comment-2, there is a UX problem here. The user has been mentioned in the 3rd comment under the 1st update. Whilst that information is present in the document fragment #update-0-comment-2 the page does not reflect that. The comments are collapsed and hidden from view.

[thisislawatts]

@davehakkens have you seen notifications that match the format you've described here?

However you already made some better/ more advanced behaviour in the howto mentions. Separating a comment from the content.

• Mentioned in a how-to
• Mentioned in a comment

Looking at the existing code I can't see anything which suggests the system is capable of generating them. This is the current implementation:

case 'howto_mention':
  return (
    <Box>
      You were mentioned in a
      <InternalLink to={notification.relevantUrl || ''}>
        how-to
      </InternalLink>
      by
      <InternalLink to={'/u/' + notification.triggeredBy.userId}>
        {notification.triggeredBy.displayName}
      </InternalLink>
    </Box>
 )

I agree that it would be good to distinguish between the different locations but perhaps that can be addressed in a follow up work item. This has already been dragging on for a while and it would be good to merge and unblock the email notifications work.

[thisislawatts]

Ready for a re-review ✨

[davehakkens]

@davehakkens have you seen notifications that match the format you've described here?

Pretty sure I did. but cant seem to find it anymore either 🤔
Perhaps I was mixing these two notifications:

For how-to author:
New comment on your how-to by projectkamp

For being mentioned in how-to comment:
You were mentioned in a how-to by projectkamp

But agree its good to release this one. Looks good like this.
One small thing I noticed (not sure its a local problem)
When you click the notification, it doesnt scroll to the correct step (but the step-comments are opened)
Video below

no.scroll.mov

[thisislawatts]

Good catch @davehakkens, I had adjusted the way we declare addresses for comments in Research articles. I've added a test to explicitly capture this behaviour. Please pull down and generate a new mention to test the behaviour. Any old notifications created locally will not work. This only impacts people who have been reviewing this branch.

[davehakkens]

works now!

[chrismclarke]

Thanks @thisislawatts , there's quite a bit here but it all makes sense to me so thanks for the addition.
I've added one (semi) blocking nit related to the regex used to check whether the page url references mentions, and then just a couple other minor bits of code tidying.

Let me know if you think you will get a chance to review these in the next week or so, if not I can just include my suggested fix for the regex and do any other tidying at some point in the future.

[chrismclarke]

Thanks for all the changes @thisislawatts, looks good to go to me!

[onearmy-bot]

🎉 This PR is included in version 1.38.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀