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.

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

Simplify the owner update queries when the instance has no one2one rooms #537

Merged
merged 6 commits into from
Jan 4, 2018

Conversation

nickvergessen
Copy link
Member

Fix #529

Signed-off-by: Joas Schilling <coding@schilljs.com>
@danxuliu
Copy link
Member

danxuliu commented Jan 3, 2018

Still failing with PostgreSQL:

An exception occurred while executing 'UPDATE "oc_spreedme_room_participants" SET "participantType" = ? WHERE "userId" <> ''' with params [2]: SQLSTATE[42703]: Undefined column: 7 ERROR: column "userId" does not exist LINE 1: ...om_participants" SET "participantType" = $1 WHERE "userId" <... ^ HINT: Perhaps you meant to reference the column "oc_spreedme_room_participants.userid".

@nickvergessen
Copy link
Member Author

nickvergessen commented Jan 3, 2018

That is quite awkward, we always used mixed cases:

$table->addColumn('userId', Type::STRING, [
'notnull' => false,
'length' => 255,
]);
$table->addColumn('roomId', Type::INTEGER, [
'notnull' => true,
'length' => 11,
]);
$table->addColumn('lastPing', Type::INTEGER, [
'notnull' => true,
'length' => 11,
]);
$table->addColumn('sessionId', Type::STRING, [
'notnull' => true,
'length' => 255,
]);

<field>
<name>userId</name>
<type>text</type>
<notnull>false</notnull>
<length>255</length>
</field>
<field>
<name>roomId</name>
<type>integer</type>
<notnull>true</notnull>
<length>4</length>
</field>
<field>
<name>lastPing</name>
<type>integer</type>
<notnull>true</notnull>
</field>
<field>
<name>sessionId</name>
<type>text</type>
<notnull>true</notnull>
<length>255</length>
</field>

I guess that means we have to replace camelcase with underscore or just lowercase everything.

I just checked and no other app tries to use camelcase. So maybe this just never worked with postgres, or it's something from a doctrine upgrade.

@Ivansss agreed?

This fixes the migrations for postgres, but the live code still errors.

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the bugfix/529/simplify-owner-query-for-empty-instances branch from c4c424e to 1b13e47 Compare January 3, 2018 15:40
@nickvergessen
Copy link
Member Author

I fixed it now, so column names are using _ instead of CamelCase.

I did some quick tests (entering a call) and all worked on postgres now. @Ivansss can you run a little test with an app against it?

Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Copy link
Member

@Ivansss Ivansss left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@Ivansss Ivansss merged commit b156e03 into master Jan 4, 2018
@Ivansss Ivansss deleted the bugfix/529/simplify-owner-query-for-empty-instances branch January 4, 2018 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants