Skip to content

Make room names non-optional#1567

Merged
nickvergessen merged 8 commits into
masterfrom
feature/noid/non-empty-room-names
Feb 28, 2019
Merged

Make room names non-optional#1567
nickvergessen merged 8 commits into
masterfrom
feature/noid/non-empty-room-names

Conversation

@nickvergessen

@nickvergessen nickvergessen commented Feb 20, 2019

Copy link
Copy Markdown
Member

Should work fine with Internal signaling already.

@nickvergessen nickvergessen added 2. developing enhancement feature: api 🛠️ OCS API for conversations, chats and participants labels Feb 20, 2019
@nickvergessen nickvergessen added this to the 💚 Next Major milestone Feb 20, 2019
'alluserids' => $room->getParticipantUserIds(),
'properties' => [
'name' => $room->getName(),
'name' => $room->getDisplayName(''),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@fancycode what do you use the name for?
Because it will be "UserA, UserB" for one-to-one rooms now, because the same message is used for both participants and I can not send different messages?

@nickvergessen
nickvergessen force-pushed the feature/noid/non-empty-room-names branch 3 times, most recently from 9524545 to e4c8731 Compare February 21, 2019 15:52

@danxuliu danxuliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests need to be adjusted.

@nickvergessen

Copy link
Copy Markdown
Member Author

Na, a rebase should be enough, let me do that.

@nickvergessen
nickvergessen force-pushed the feature/noid/non-empty-room-names branch from 07e6cf0 to b6bd023 Compare February 26, 2019 09:52
@nickvergessen

Copy link
Copy Markdown
Member Author

And pushed a fix to add names to all the integration rooms

@Ivansss Ivansss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works 👍

@nickvergessen nickvergessen mentioned this pull request Feb 26, 2019
11 tasks
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
because mobile apps and web use it to get the avatar

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessen force-pushed the feature/noid/non-empty-room-names branch from 7f276b1 to 2af378f Compare February 27, 2019 14:27
@nickvergessen

Copy link
Copy Markdown
Member Author

Rebased to fix the conflict in UserMentionsTest

nickvergessen and others added 2 commits February 27, 2019 17:43
Signed-off-by: Joas Schilling <coding@schilljs.com>
Now there is no longer a "New group conversation" item shown in the
conversation list dropdown; instead first the desired conversation name
needs to be typed in the search input and then the group conversation
with that name is selected in the dropdown.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu
danxuliu force-pushed the feature/noid/non-empty-room-names branch from d4f445a to ddae520 Compare February 27, 2019 17:37
Although the server will refuse to set an empty room name "validate" was
implemented in "Room" model, which prevents "Room.save" from even
sending the empty name to the server.

The EditableTextLabel now checks too if an "invalid" event was triggered
when calling "Model.save", and if that happens it simply hides the input
showing again the label without changes; it would be better to keep the
input and show the error message instead, but for now it is good enough.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu

Copy link
Copy Markdown
Member

I have squashed the fixes for the unit and acceptance tests.

Besides that, I added a commit to prevent saving an empty name for a room.

@danxuliu danxuliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and works; empty names in existing rooms are nicely converted to the list of users 👍

I think that the UX to create a new group or public room needs further work, but we can do that later.

@nickvergessen
nickvergessen merged commit e3efeb1 into master Feb 28, 2019
@nickvergessen
nickvergessen deleted the feature/noid/non-empty-room-names branch February 28, 2019 07:45
@nickvergessen nickvergessen mentioned this pull request Apr 10, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement feature: api 🛠️ OCS API for conversations, chats and participants

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants