Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

refractor: update code and project to use latest Appwrite SDK #23

Merged
merged 21 commits into from Aug 2, 2022
Merged

refractor: update code and project to use latest Appwrite SDK #23

merged 21 commits into from Aug 2, 2022

Conversation

kekavc24
Copy link
Contributor

Saw the project was last updated in May and is part of the Awesome Appwrite Project. Decided to help migrate and update it. I refractored some code and made some changes.

Changes Made

  • Upgraded the Appwrite SDK to latest version in pubspec.yaml
  • Added missing flutter lints and included new equatable package
  • Refractored various parts of the code to make code look cleaner
  • Fixed some linter errors and errors shown as a result of upgrading packages

@@ -2,4 +2,5 @@ class AppConstsnts {
static const String endPoint = "https://demo.appwrite.io/v1";
static const String project = "606d5bc9de604";
static const String collection = "606d5bd9626ae";
static const String databaseID = "";

Choose a reason for hiding this comment

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

Should be "default" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know the database ID for tutorial. Lemme apply this in a few

Database db = Database(client);
final collectionId = 'quiz_questions';
final res = await db.createCollection(
Databases db = Databases(client, databaseId: 'unique()');

Choose a reason for hiding this comment

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

Maybe this one should be default also to match up with the other value after created

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this commit today morning after I read the discussion @stnguyen90 had with @2002Bishwajeet concerning how to create the databases with Serverside SDK. You have to provide a "unique()" parameter first and then provide the actual id to be used. If I'm not wrong since the constructor has to be there 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I change it to default instead of Quizy ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Putting unique() and then calling db.create(name: 'Quizy'); would create a Database with a unique auto generated ID, but you can also use your own custom ID to create the Database with that ID.

I agree with @brandonroberts suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll fix that. Thanks for the clarification. Seems I got confused as you explained.

final res = await db.createCollection(
Databases db = Databases(client, databaseId: 'unique()');

db.create(name: 'Quizy'); // added funky name

Choose a reason for hiding this comment

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

Should this be using await?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it should. I'll fix this.

load_questions.dart Outdated Show resolved Hide resolved
Co-authored-by: Brandon <robertsbt@gmail.com>
@brandonroberts brandonroberts changed the title Refractor Code and Upgrade project to use latest SDK refractor: update code and project to use latest Appwrite SDK Jul 27, 2022
Copy link

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

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

LGTM

load_questions.dart Outdated Show resolved Hide resolved
load_questions.dart Show resolved Hide resolved
lib/constants.dart Outdated Show resolved Hide resolved
@kekavc24 kekavc24 requested a review from stnguyen90 July 30, 2022 10:05
@@ -1,5 +1,6 @@
class AppConstsnts {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename this too. Ofc this doesn't make sense.😹

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, looks like you need to merge the latest changes from upstream. A PR has been already made, 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅 fixed after some few issues

lib/quiz.dart Outdated
@@ -65,16 +73,16 @@ class _QuizPageState extends State<QuizPage> {
itemCount: questions!.length,
itemBuilder: (context, index) {
final question = questions![index];
return Container(
return SizedBox(
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove SizedBox also, I don't think we should be needing that here.

@@ -107,13 +117,13 @@ class _QuizPageState extends State<QuizPage> {
},
),
)
: Container(
: const SizedBox(
child: Text("Some error! Check console"),
),
/* A language that doesn't affect the way you think about programming is not worth knowing. */
Copy link
Contributor

Choose a reason for hiding this comment

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

It's outdated now. You can remove these comments too😆

lib/quiz.dart Outdated
@@ -107,13 +117,13 @@ class _QuizPageState extends State<QuizPage> {
},
),
)
: Container(
: const SizedBox(
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, unnecssary SizedBox here

@stnguyen90
Copy link
Contributor

LGTM!

Copy link

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

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

LGTM

@brandonroberts brandonroberts merged commit d4caca1 into appwrite:main Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants