Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Have editMeetingTimes use same funcs as createMeetingTimes #1592

Merged
merged 1 commit into from
Feb 27, 2024

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

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

@@ -41,13 +41,10 @@ const EditMeetingTimes = ({
};
}

//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) {

@@ -63,50 +60,23 @@ const EditMeetingTimes = ({
updatedDate,
};

// 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.


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
6 checks passed
@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