submission commit

[0lcm]

No description provided.

[hasona23]

Hi @0lcm
Hope you are doing fine. I'll start reviewing your e-commerce API app. hope you learned a lot from it.
Will post review soon. 😄

bye 👋

[0lcm]

Hi @hasona23, I noticed that I had accidentally forgotten to commit a couple migrations during development, so I pushed them just now. Thank you taking the time to review my project!

[hasona23]

Hi @0lcm

Review point:
1- Critical 🔴 :

1- Price Validation:

2- Check for duplicate project tags

3- I don't understand why are tags stored if i cant display them as list and search by tag nor add 1 to product without typing name manually

4- Sales Reivew produces this error

5- Same error happened with adding sale

6- Same error happens with also deleting Sales

7- There is an error without pagination/get items logic which doesn't happen in the Tag system. the error is duplicate data i thought it was database but the data in api.db is fine and not duplicated (and ofcourse sqlite wont duplicate id)

2- Warning 🟡:
1- Solution/Repository naming: it is advised to name as [project name].[GitHub name]
2- Pages system doesn't handle the edge case of an empty page in 2 ways:
- Not removing Next page button
- Allowing to go to empty page

3- Don't make user have to memorize id of products to delete as this will get even more complex when it is not incrementing integer ID (1,2,3,4) but hash id (5f4dcc3b5aa765d61d8327deb882cf99).
instead you could either
- Display them all like in list or with more brief information and then prompt for id
- Use a selection prompt (single or multi up to you)

4- Back option in delete product doesn't work the "null argument" text shouldn't be displayed

5-

you need to make it case-in-sensitive and make sure

6- It is better to not clear any of the input data until all is entered dont clear after each input paramter so i could keep track of what i enterd up till now

6,7- Also for search to leave it even with displaying data so that i could know what is the search for or just type what i set for the search paramters

6,8- For add could allow me

9- It is better and mostly common way to put it in the appsettings.json file any config paramters should go there you could even see it in the academy repositoy although it is not sent through github but reading the README implies the use of it

3- Suggestion 🔵 :
1- Don't clear screen in searching products section till i enter my search paramters

2- Dont delete screen until I finished adding all Create product parameters

3- Display Products as a table instead of list it will cover less vertical space

4- It is better to make the enum displayed as a selection prompt instead of text prompt. it is easier for both you and the as user doesnt have to take care of spelling errors or hidden characters and you will have less headache validating what he wrote

4- Awesome 🟢
1- Pagination
2- Storing constant reused and some config data as constants and static such as Utils.cs and UiHelper.cs
3- The amazing use of Spectre.Console functions which most people ignore. although will benefit from adding slightly more colors and reduce/moderate the use of clear console

I Still didnt finish reading the whole code but the Critical 🔴 must be done as the project wont be accepted till then.
I must say it is amazing all the stuff you did including pagination and your first semi-working API with ui Using Spectre console

I know this may seem daunting at first but it is your first time so this actually quite normal😄

looking forward to seeing your adjustments 😄
Good luck!

[hasona23]

Critical 🔴 :

1. Price can be set to zero. it must be non-negative nor zero

2) Something wrong with title header display 3) Cant find the products i made to delete. perhaps there is missing pagination or something 4) Empty page in tag listing 5) Empty infinite pages number in sales Moderate 🟡 : 6) Just prompting for ID in delete sales and tags would be better to do like in products. You should think about how user feels when using the app is it good for him to keep searching for products and find and memorize their id

You are so close to making it
Keep up the good work 👍

[hasona23]

The project is functional and meets the criteria.

1-

I think it is better to have products displayed vertically to handle having lots of products and also have a border to seperate the rows

Code Review:
1- API
1-

usually you store the connection string in appsettings.json file then add this line to your program.cs file `var connectionString = builder.Configuration.GetConnectionString("DefaultConnection");` however make sure you dont push it to public respository if you are doing a real application. this should be hidden 2- It is wonderful that you used Swagger for your API. you can also check Scalar it is similar to swagger but more customizable and has better UI

3- Usually you want to seed data if the DB is empty so you can check instead of isFirstRun check if the database has desired data by using context.Sales.Any().

4- Good use of error handling. you can add to it the use of a logger. you are correct about not showing error details to user since this is unhandled error but another step you could add is logging this error weather to database where you store logs to later see it and in the logger you will log the error with details such as exception message and location.

5- It is good that you used Interceptors for soft deletion. sometimes there is also a soft-update basically similar to soft-delete where you store the time the model was last updated

6- Seeding data is working fine. could to it seeding for sales too

7- You could use the result pattern for return types of IService/IRepository instead of depending on bool? or Item? which may cause confusion at some point weather for you or your team mates later when you work on bigger projects. you can use record/class/struct and it is easy to implement. you have 2 main types Result and Result, where Result has a bool success and string error message(incase of failure) and Result has same features but with addition of T? Data{get;set;}
Lastly you can have static methods such as Result.Success() or Result.Fail(string msg)
Lastly this allows the api consumer to easily know the error instead of a vague InternalServerError where he doesnt know if he or the server made the issue

8. Usually you separate it into Services and Repositories folder instead of a generic interfaces folder. you already have those folders so just move the interfaces into the same folder or create an Interfaces sub-folder inside the Repository and Services folder instead of both being mixed in a single dir

9. 🔴 I noticed you have a Critical Security issue of validation

validation could be done in your controller API Primarily and optionally at your UI layer too. C# Asp.net API makes this easy to implement actually. there is an attribute called [ApiController] this automatically validates incoming models. other wise like in MVC later on you will have to use ModelState.IsValid.
Ok where does the validation logic resides? You use attributes as demonstrated by the below image. it can cover almost all your use cases from int,float,double,string etc

2- CLI / UI

1. Make class static

2- Same thing as in shared

3. Same as in API

4. The naming is a little vague to me since you dont have any AUTH implemented or users so there is no admin/normal user.

3- SHARED
It is good that you use Shared project and a good practice 👏 . There is a tiny enhancement you could do which is make a folder called Enums and inside have all your enums such as ItemFormat.cs and ItemType.cs instead of a universal Enum.cs file

this was the stuff that caught my attention in your code. your code is really good and even the program.cs file in the UI project is advanced but works well.
Good luck on the next project and sorry for taking long to review code
I'll close this PR and mark this project as complete
Well Done 👏 🎉 👏 🎉 👏 🎉