Skip to content

Add Coding tracker project - #278

Open
jcisco22 wants to merge 1 commit into
the-csharp-academy:mainfrom
jcisco22:main
Open

jcisco22 wants to merge 1 commit into
the-csharp-academy:mainfrom
jcisco22:main

Conversation

@jcisco22

Copy link
Copy Markdown

No description provided.

@TheCSharpAcademy

Copy link
Copy Markdown
Collaborator

@jcisco22 Thanks for submitting! I can't see this project available for review. Could you please submit in the dashboard before we can approve 😁.

@github-actions

Copy link
Copy Markdown

🚀 Project Submitted!

Thanks for submitting your project! I will review it soon.

📋 Please check the dashboard to make sure the project is marked as Pending.

⚠️ If it's not marked as pending, submit it manually using the URL for this PR.

@TheCSharpAcademy TheCSharpAcademy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@jcisco22 Thanks for submitting! Before we approve, please fix these important bugs:

❌ Duration isn't kept correct when updating a record. You correctly calculate duration during insertion:

string duration = (endTime - startTime).ToString();

But Update() only updates StartTime and EndTime:

UPDATE coding_tracker
SET StartTime = @StartTime,
    EndTime = @EndTime
WHERE ID = @Id

This means an updated record can display its old duration with its new start/end times.

❌ End time can currently be earlier than start time. This is specifically called out in the project's validation tips. At the moment both times are individually valid, but there's no validation of their relationship.

For example, the program accepts:

Start: 18:00
End:   15:00

and will calculate a negative duration.

💁‍♂️If you have any questions, the best way to get help is in our Discord, in the #get-help-with-projects channel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants