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

avoid use of sql to migrate table #125

Closed
wants to merge 1 commit into from

Conversation

sualko
Copy link

@sualko sualko commented Jan 31, 2022

I did not test it, but at least this is what the api provides. I also added a type hint for table so vsc shows you some documentation.

fix #123

@baimard
Copy link
Owner

baimard commented Jan 31, 2022

Dear @sualko,

  • don't push on master
  • Please test before PR

Are you that this change the name of column siret in legal_one?
Could you please share where you find that in the documentation ?

        if ($table->hasColumn('siret')) {
            $table->changeColumn('siret', [
                'type' => 'longtext',
            ]);

            $table->changeColumn('legal_one', [
                'type' => 'longtext',
            ]);

@baimard baimard added the question Further information is requested label Jan 31, 2022
@baimard baimard self-assigned this Jan 31, 2022
@baimard baimard marked this pull request as draft January 31, 2022 21:54
@sualko
Copy link
Author

sualko commented Jan 31, 2022

don't push on master

Is this documented somewhere? In the Nextcloud universe the most common pattern is that the main branch is for development, because most people will open a pr for that branch.

Are you that this change the name of column siret in legal_one?

Sorry, I read this falsely. If you want to rename it and change the type, you should maybe check the manual for a more error prone method: https://docs.nextcloud.com/server/latest/developer_manual/basics/storage/migrations.html

Could you please share where you find that in the documentation?

In the Doctrine documentation. If you open my pr in VSC you will have autocomplete for the table variable, since I added a type hint as described above.

@baimard
Copy link
Owner

baimard commented Jan 31, 2022

The problem of #123 is the ALTER TABLE.

According to the doctrine documentaiton, we need to use :

// ...
public function up()
{
    $this->renameTable( 'old_table_name', 'new_table_name' );
}
// ...

But I don't know why that not work in nextcloud. I'm working on this issue.

@sualko
Copy link
Author

sualko commented Jan 31, 2022

$this->renameTable( 'old_table_name', 'new_table_name' );

This will rename the table, but you want to rename a column.

@baimard
Copy link
Owner

baimard commented Jan 31, 2022

$this->renameTable( 'old_table_name', 'new_table_name' );

This will rename the table, but you want to rename a column.

it's certainly why that not work :)

// ...
public function up()
{
    $this->renameColumn( 'table_name', 'old_column_name', 'new_column_name' );
}
// ...

@baimard baimard closed this Jan 31, 2022
@baimard baimard mentioned this pull request Feb 3, 2022
baimard added a commit that referenced this pull request Feb 3, 2022
* refactoring

* refactoring

refactoring

refactoring & #46

Create help section for configuration and #47 #61

changelog

[tx-robot] updated from transifex

Signed-off-by: Nextcloud bot <bot@nextcloud.com>

[tx-robot] updated from transifex

Signed-off-by: Nextcloud bot <bot@nextcloud.com>

[tx-robot] updated from transifex

Signed-off-by: Nextcloud bot <bot@nextcloud.com>

[tx-robot] updated from transifex

Signed-off-by: Nextcloud bot <bot@nextcloud.com>

* transifex

* transifex

* test

* Translation

* translation

* hotfix

* #123

* Update lib/Migration/Version20002Date20220201085856.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version20002Date20220201085856.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version7Date20220125174818.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version7Date20220125174818.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version7Date20220125174818.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* #135

* Update lib/Migration/Version20002Date20220201085856.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* valide

* valide

* final

* Final

* Final

* Update lib/Migration/Version20002Date20220201085856.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version20002Date20220201085856.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version7Date20220125174818.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version7Date20220125174818.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* Update lib/Migration/Version7Date20220125174818.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* [tx-robot] updated from transifex

Signed-off-by: Nextcloud bot <bot@nextcloud.com>

* l10n: Fixed grammar

Reported at Transifex.

Signed-off-by: rakekniven <2069590+rakekniven@users.noreply.github.com>

* l10n: Improve styling

A more selected description of the function setting.

* l10n: Improve styling 

A more selected description of the function setting.

* l10n: Add "an"

* l10n: Add "an"

* l10n: Replace a word

* l10n: Replace a word

* [tx-robot] updated from transifex

Signed-off-by: Nextcloud bot <bot@nextcloud.com>

* Update lib/Migration/Version20002Date20220201085856.php

Co-authored-by: Vitor Mattos <vitor@php.rio>

* valide

* valide

* final

* Final

* Final

Co-authored-by: Vitor Mattos <vitor@php.rio>
Co-authored-by: Nextcloud bot <bot@nextcloud.com>
Co-authored-by: rakekniven <2069590+rakekniven@users.noreply.github.com>
Co-authored-by: Valdnet <47037905+Valdnet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EN] SQL exception during update
2 participants