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.

Sign up for GitHub

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

Jump to bottom

add post to conversation flow operator #2487

Merged
merged 12 commits into from
Dec 16, 2019
Merged

add post to conversation flow operator #2487

merged 12 commits into from
Dec 16, 2019

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Nov 22, 2019 •
edited
Loading

for #2414

fyi @juliushaertl @nickvergessen

@blizzz blizzz added this to the 💚 Next Major (18) milestone Nov 22, 2019
@nickvergessen nickvergessen self-requested a review November 26, 2019 12:55
nickvergessen
nickvergessen reviewed Nov 26, 2019
View reviewed changes
lib/Flow/Operation.php Outdated Show resolved Hide resolved
lib/Flow/Operation.php Outdated Show resolved Hide resolved
lib/Flow/Operation.php
try {
$participant = $this->getParticipant($room);
if (!$participant->hasModeratorPermissions(false)) {
throw new UnexpectedValueException('Not allowed to mention room');
Copy link
Member

Choose a reason for hiding this comment

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

Is this intended? we do not limit mentions of the "conversation" to moderators

Copy link
Member Author

Choose a reason for hiding this comment

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

We agreed that we do not want a person be able to spam others with mentions. Now I had the idea that some trusted individuals it might be allowed to trigger mentions to a crowd, and in Talk they would be identified by Moderators. If it does not make sense, we can kill it. Or extend/relax it later.

src/PostToConversation.vue Outdated Show resolved Hide resolved
src/PostToConversation.vue Outdated Show resolved Hide resolved
src/PostToConversation.vue Outdated Show resolved Hide resolved
src/PostToConversation.vue Outdated
this.fetchRooms()
},
methods: {
fetchRooms() {
Copy link
Member

Choose a reason for hiding this comment

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

There is a RoomSelector.vue available for the projects stuff. Would it make sense to reuse?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll have a look, thx for the pointer

Copy link
Member Author

Choose a reason for hiding this comment

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

Only with refactoring the RoomSelector as it brings a hard model with it. I'd refrain of it for now.

src/flow.js Outdated Show resolved Hide resolved
@blizzz blizzz requested a review from nickvergessen November 27, 2019 16:23
@blizzz blizzz mentioned this pull request Nov 29, 2019
Merged
@blizzz blizzz force-pushed the enh/2414/flow-operator branch from 54359ca to a250cad Compare November 29, 2019 10:29
@blizzz blizzz marked this pull request as ready for review November 29, 2019 10:29
blizzz added 10 commits December 13, 2019 16:02
@blizzz
add flow operation, backend part
8e81d70 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
add flow operation, frontend part
c6baf40 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
prepare mention of the message
34537df 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
catch Talk exceptions
8e042b5 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
move mode constants to constants
a0dd6ef 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
mv to views/FlowPostToConversation.vue
8815635 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
use acting user from Operation config
f36f59f 
because the logged in user might not be the one who set up the flow, e.g.
a share recipient

Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
fix user mentions on uids without whitespace
c49f796 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
show a nice message, if the entity offers one
6258d21 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
take a URL into consideration, if the entity provides one
28a3e31 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz force-pushed the enh/2414/flow-operator branch from a250cad to 28a3e31 Compare December 13, 2019 15:05
@blizzz
satisfy linter
ecd3ce8 
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz
adjust to server changes
2a87d67 
Signed-off-by: Arthur Schiwon <[email protected]>
nickvergessen
nickvergessen approved these changes Dec 16, 2019
View reviewed changes
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Works 🐘

@nickvergessen nickvergessen merged commit 32049dc into master Dec 16, 2019
@nickvergessen nickvergessen deleted the enh/2414/flow-operator branch December 16, 2019 14:24
@blizzz blizzz mentioned this pull request Dec 17, 2019
Merged
@danxuliu danxuliu mentioned this pull request Dec 18, 2019
Merged
@juliusknorr juliusknorr mentioned this pull request Dec 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliusknorr juliusknorr juliusknorr left review comments

@nickvergessen nickvergessen nickvergessen approved these changes

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

@danxuliu danxuliu Awaiting requested review from danxuliu

Assignees

@blizzz blizzz

Labels
3. to review enhancement
Projects
None yet
Milestone
v8.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@blizzz @nickvergessen @juliusknorr