Skip to content

feat: Have editMeetingTimes use same funcs as createMeetingTimes#1592

Merged
trillium merged 1 commit intohackforla:developmentfrom
trillium:ts.frontEndTimingBug
Feb 27, 2024
Merged

feat: Have editMeetingTimes use same funcs as createMeetingTimes#1592
trillium merged 1 commit intohackforla:developmentfrom
trillium:ts.frontEndTimingBug

Conversation

@trillium
Copy link
Member

@trillium trillium commented Feb 26, 2024

Fixes #1591

What changes did you make and why did you make them ?

  • the update function in prod/dev creates a new date timestamp and assigns it to date on edit, messing up timing
  • Helper functions for createMeetingTimes generate clean date timetamps
  • Use helper functions from createMeetingTimes to generate the right timing for event check in

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

Before dev.vrms.io update -- See raw.date is a clean time stamp with 00:00

image

After dev.vrms.io update -- See raw.date is not a clean time stamp with 00:00

image
Visuals after changes are applied

After a re-update from new branch See raw.date is again a clean time stamp with 00:00

image

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b Spiteless-ts.frontEndTimingBug development
git pull https://github.com/Spiteless/VRMS.git ts.frontEndTimingBug

}

//if there is a description or a blank description
if (values.description || !values.description) {
Copy link
Member Author

@trillium trillium Feb 26, 2024

Choose a reason for hiding this comment

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

This portion of the code always evaluates to true, removed if block

Copy link
Member

Choose a reason for hiding this comment

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

Lolol

Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should be

if (values.description) {


// If the day has been changed, find the next occurence of the changed day
if (values.day) {
const date = findNextOccuranceOfDay(values.day);
Copy link
Member Author

Choose a reason for hiding this comment

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

This portion of the code is what creates a false time stamp. values always has a .day property in the front end, so the code assumes we are needing tom update the day.

This is the most relevant changes to this PR

we use the previous time or duration for the calculation.
*/
// Find next occurance of Day in the future
// Assign new start time and end time
Copy link
Member Author

Choose a reason for hiding this comment

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

In createMeetingTimes.js the timing selections for a meeting are used to create a correct meeting time. We have access to the same information and it has been consistently correct, this is the most correct information we have about the meeting times.

const startTimeDate = timeConvertFromForm(date, values.startTime);
const endTime = addDurationToTime(startTimeDate, values.duration);

let startTimeToUse = startTimeOriginal;
Copy link
Member Author

Choose a reason for hiding this comment

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

Getting rid of the old timing code

Copy link
Member

@jbubar jbubar left a comment

Choose a reason for hiding this comment

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

I tested it out on my end. this looks good. I am hoping it works for fixing the bug

@trillium trillium merged commit 8a9bafd into hackforla:development Feb 27, 2024
@trillium trillium deleted the ts.frontEndTimingBug branch February 27, 2024 03:33
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.

Stand in ticket for checkIn issue.

2 participants