Louvorg/ReaderForSelfoss-multiplatform
Louvorg/ReaderForSelfoss-multiplatform
3
2
Fork 2
Code Issues 18 Pull Requests Actions Projects 1 Releases 65 Wiki Activity

Make the author field nullable #117

Merged
AmineB merged 3 commits from davidoskky/ReaderForSelfoss-multiplatform:author into master 2022-12-28 14:25:38 +00:00
Conversation 2 Commits 3 Files Changed 5 +12 -5
davidoskky commented 2022-12-28 10:03:53 +00:00
Contributor
Copy Link

Types of changes

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This is NOT translation related.

The author field is nullable, these changes will handle it and prevent crashes.

## Types of changes - [x] I have read the **CONTRIBUTING** document. - [x] My code follows the code style of this project. - [x] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [x] All new and existing tests passed. - [x] This is **NOT** translation related. The author field is nullable, these changes will handle it and prevent crashes.
davidoskky added 1 commit 2022-12-28 10:03:54 +00:00
Make the author field nullable
All checks were successful
continuous-integration/drone/pr Build is passing
Details
5227751dca
AmineB requested changes 2022-12-28 10:56:48 +00:00
shared/src/commonMain/sqldelight/bou/amine/apps/readerforselfossv2/dao/1.sqm Outdated
@ -1 +1 @@
ALTER TABLE ITEM ADD COLUMN `author` TEXT NOT NULL;
ALTER TABLE ITEM ADD COLUMN `author` TEXT;
AmineB commented 2022-12-28 10:56:30 +00:00
Owner
Copy Link

You can´t update a sqldelight migration file. You have to create a new file 2.sq and add the sql query changing the column inside.

You can´t update a sqldelight migration file. You have to create a new file `2.sq` and add the sql query changing the column inside.
davidoskky commented 2022-12-28 10:58:21 +00:00
Author
Contributor
Copy Link

Ops, I thought this version hadn't been released yet and that thus that would be fine.

Ops, I thought this version hadn't been released yet and that thus that would be fine.
AmineB commented 2022-12-28 12:18:56 +00:00
Owner
Copy Link

I just saw that you can't update a column type in sqlite.

You'll have to rename the existing one create a new one with the old name and the right type, and then drop the old one.(not possible in older versions of android because it's only available since version 3.25.0 of sqlite, and Android has old versions)

You'll have to drop the old column, and recreate it.

Will you be able to do these changes today ? This seem to cause issues for multiple users.

I just saw that you [can't update a column type in sqlite](https://www.sqlite.org/lang_altertable.html#alter_table_rename_column). ~~You'll have to rename the existing one create a new one with the old name and the right type, and then drop the old one.~~(not possible in older versions of android because it's only available since version 3.25.0 of sqlite, and Android has [old versions](https://stackoverflow.com/a/52346199)) You'll have to drop the old column, and recreate it. Will you be able to do these changes today ? This seem to cause issues for multiple users.
davidoskky commented 2022-12-28 13:15:07 +00:00
Author
Contributor
Copy Link

Yes, I'll submit everything soon.

Yes, I'll submit everything soon.
👍 1
davidoskky added 2 commits 2022-12-28 13:26:07 +00:00
Include author field when updating the database
All checks were successful
continuous-integration/drone/pr Build is passing
Details
e51915d1cd
AmineB requested changes 2022-12-28 13:32:34 +00:00
AmineB left a comment
Owner
Copy Link

Can you please change the migration to a simpler one ?

Can you please change the migration to a simpler one ?
shared/src/commonMain/sqldelight/bou/amine/apps/readerforselfossv2/dao/2.sqm
@ -0,0 +1,6 @@
CREATE TABLE ITEM_BACKUP AS SELECT `id`, `datetime`, `title`, `content`,
AmineB commented 2022-12-28 13:32:15 +00:00
Owner
Copy Link

This is unnecessary.

The data will be updated if needed with the update of updateItem you made.

Droping the column and recreating it should by enough.

This is unnecessary. The data will be updated if needed with the update of `updateItem` you made. Droping the column and recreating it should by enough.
davidoskky commented 2022-12-28 13:38:45 +00:00
Author
Contributor
Copy Link

Dropping columns was introduced in version 3.35 https://sqlite.org/changes.html

Dropping columns was introduced in version 3.35 https://sqlite.org/changes.html
👍 1
davidoskky commented 2022-12-28 13:34:13 +00:00
Author
Contributor
Copy Link

Alright, this should now work; I'm not able to test the migration because the previous version crashes on me, check that everything works smoothly.
I had to drop the whole table since also renaming columns is a new feature.

Alright, this should now work; I'm not able to test the migration because the previous version crashes on me, check that everything works smoothly. I had to drop the whole table since also renaming columns is a new feature.
AmineB commented 2022-12-28 14:25:23 +00:00
Owner
Copy Link

I just realized that migrations aren't run automatically, son the first one wasn't done. I'll merge this and make a small change so it would work as expected.

I just realized that migrations aren't run automatically, son the first one wasn't done. I'll merge this and make a small change so it would work as expected.
👍 1
AmineB merged commit a094d88799 into master 2022-12-28 14:25:38 +00:00
davidoskky deleted branch author 2022-12-28 14:28:04 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
AmineB
Labels
Clear labels   
Difficulty - Beginner friendly

   
Difficulty = Easy

   
Difficulty = Hard

   
Difficulty = Medium

   
Priority = CRITICAL

   
Priority = High

   
Priority = Low

   
Priority = Normal

   
Priority = Some day

   
Status - Fixed somewhere else

   
Status - No details provided

   
Status = Can't fix

   
Status = Duplicate

   
Status = Help wanted

   
Status = Invalid

   
Status = Need more details

   
Status = Taken

   
Status = Wontfix

   
Type - Selfoss works like this

   
Type = Bug

   
Type = Chore

   
Type = Enhancement

   
Type = Feature

   
Type = Question

   
Type = SELFOSS API ISSUE

   
Type = Tools

   
Type = UX/Design

   
Up For Grabs
No Label
Difficulty - Beginner friendly
Difficulty = Easy
Difficulty = Hard
Difficulty = Medium
Priority = CRITICAL
Priority = High
Priority = Low
Priority = Normal
Priority = Some day
Status - Fixed somewhere else
Status - No details provided
Status = Can't fix
Status = Duplicate
Status = Help wanted
Status = Invalid
Status = Need more details
Status = Taken
Status = Wontfix
Type - Selfoss works like this
Type = Bug
Type = Chore
Type = Enhancement
Type = Feature
Type = Question
Type = SELFOSS API ISSUE
Type = Tools
Type = UX/Design
Up For Grabs
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Louvorg/ReaderForSelfoss-multiplatform#117
No description provided.
Delete Branch "davidoskky/ReaderForSelfoss-multiplatform:author"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?