codefordenver / Comrad

Open-source web application for radio stations to manage show schedules, traffic and compliance
ISC License
25 stars 9 forks source link

When updating a traffic event with a new end date, the end date doesn't save #834

Closed seankwilliams closed 1 year ago

seankwilliams commented 1 year ago

Steps to replicate:

  1. Go to Traffic in the left menu
  2. Click Add
  3. Add an underwriting event. Set it to repeat, but do not provide an end date:

image

  1. Save the event
  2. Go back to the event and click Edit. Choose to Edit Series.
  3. Add an end date
  4. Click Save
  5. Now, go back to the traffic event and choose to edit the series again. The end date will not have saved.
kmid5280 commented 1 year ago

@seankwilliams On my end, it appears that the form is not updating the dates at all. It will neither update the start nor end date if new values for them are entered on the form.

I put some console logs in server/v1/controllers/events/update.js and did a few test updates. The updated date is displaying correctly in the request, which you can see if you console.log the request body. However the correct updated date in the request does not appear to be getting passed into the updated data. The console event that says "updating event" on line 322 still shows the old date. So it looks to me like there is a disconnect here between the data being passed in and the data that is being updated in the PUT request. Thoughts?

seankwilliams commented 1 year ago

Nice research there, @kmid5280 ! It seems that this code in server/v1/controllers/events/update.js, which is supposed to set the end date for the repeat rule, may not be working right then:

image

I'd review that to see if that's related to the new repeat end date not being set!

kmid5280 commented 1 year ago

@seankwilliams A couple of things I'm noticing. There is this code on line 265:

body.repeat_rule = originalEvent.repeat_rule;

If I run console.log(body.repeat_rule) both before and after this line, I can see that the correct updated value reverts back to the original value after the line. It looks like this is setting the value of body.repeat_rule back to the original pre-updated value. However I will note that commenting this line out doesn't make any difference.

I also see that line 279 has the following:

if ('repeat_rule.repeat_end_date' in body) {

This block does not fire off if I attempt to update the date, however it will fire off if I change the line to if (body.repeat_rule['repeat_end_date') . This doesn't fix the problem and is causing some other issues, but I'm wondering if this condition is written correctly given that normally we are using bracket notation to retrieve this value. Any thoughts?

seankwilliams commented 1 year ago

@kmid5280 It seems like you're looking in the right place for the problem! I will say that a lot of the database code can get tricky because as far as Mongo is concerned, body['repeat_rule.repeat_end_date'] is the same as body['repeat_rule']['repeat_end_date'], but they are different in JavaScript.

kmid5280 commented 1 year ago

@seankwilliams In update.js, can you help me understand why we are setting the value of body.repeat_rule to the value of originalEvent.repeat_rule in lines 265 and 268-270? One thing I'm noticing is that by the time the request makes it to the conditional block on line 279, the value of the date has already reverted back to the original value. I can fix this by commenting out those blocks of code, but I want to better understand why they are there and if we need them.

seankwilliams commented 1 year ago

@kmid5280 I'm not entirely sure why that is there. My first thought is that it's there to preserve options that may not get passed in from the front-end -- for example, if the user is only changing one aspect of the repeat logic. That repeat logic is complicated because all the possible values ("Every Weekday", "Second Tuesday of Every Month", etc.) all correspond to different repeat rule JSON. Not only does the right JSON have to get saved in the database, but the front-end has to interpret that JSON to know whether to display one of the predetermined values like Every Weekday, or Custom.

If you opt to comment out the code that pulls from the original event's repeat rule, I'd just test different combinations of changing the repeat rule to be sure it's still working.