Opened 10 years ago

Closed 10 years ago

#7080 closed defect (fixed)

More nits with new schedule editor

Reported by: gigem Owned by: Isaac Richards
Priority: minor Milestone: 0.22
Component: MythTV - General Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

These are probably all cause by the same underlying bug.

Case 1:

Create a single record and choose preview without saving. Works.

Save single record.

Edit single record, change to not record and choose preview without saving. Doesn't work -- shows schedule not affected.

Case 2:

Create a record all and choose preview without saving. Doesn't work -- only shows one program will record.

Case 3:

Create a record all and save.

Edit record all, change to record one and save. Rule is saved correctly, but a reschedule is not done. The effect of the change is not seen until a reschedule is caused by some other action.

Change History (11)

comment:1 Changed 10 years ago by stuartm

(In [21920]) Store the rule type as soon as the value changes so that the preview is working with the right information. Should fix the first two cases in #7080, but I haven't checked. Refs #7080

comment:2 Changed 10 years ago by stuartm

Milestone: unknown0.22
Status: newinfoneeded_new
Version: unknownhead

David, first two are fixed but for the third I need some advice. Firstly the reschedule does occur and the UI even receives the SCHEDULE_CHANGE event.

I think the problem may be the type of reschedule that we request, it's always a targeted reschedule with the recordid given. Is that a problem when we are changing from a record all to a record single? If so what other changes should always trigger a full reschedule?

comment:3 in reply to:  2 Changed 10 years ago by gigem

Status: infoneeded_newnew

Case 2 now works, but case 1 still doesn't work for me.

Yes, the reschedule type is wrong. It should be for the specific recordid that is changed. Your sending one for the magic recordid 0 which means none of the rules changed. A reschedule for recordid 0 is typically done when a recording ends or previously recorded is changed so rsPrevious and rsCurrent recording statuses get updated.

comment:4 Changed 10 years ago by stuartm

No, we never send a reschedule for recordid 0 - if that's happening it's a bug. recordid shouldn't even be zero if you are editing an existing rule as in case 3.

I must have mis-read case 1, because you are right, it doesn't work but I can't see why not unless it was a pre-existing bug in viewscheddiff (which hasn't been modified), we insert the correct type into record_tmp. Did it even allow you to preview changes with the old editor when the type was changed to "Don't record"?

comment:5 Changed 10 years ago by stuartm

(In [21923]) Only use last_insert_id when inserting a new record, fixes reschedules for changes to existing rules. Refs #7080

comment:6 Changed 10 years ago by stuartm

Ok, that fixes Case 3. Only case 1 remains a mystery.

comment:7 in reply to:  4 Changed 10 years ago by gigem

Replying to stuartm:

Did it even allow you to preview changes with the old editor when the type was changed to "Don't record"?

That's a good question. I'll see if I can easily revert my development/semi-production version and try it. FWIW, previewing any type change (not just to no record) of an existing rule doesn't work with the current code.

comment:8 Changed 10 years ago by gigem

(In [21926]) Fix the scheduler to not let kNotRecording rules fall through the cracks and act like kSingleRecords.

When previewing the case of deleting an existing recording rule, the schedule editor saves the updated rule as kNotRecording instead of deleting the rule. This change makes the scheduler return the right results in either case. It might still be desirable for the schedule editor to delete the rule, but I'll let stuartm handle that.

Refs #7080. M scheduler.cpp

comment:9 Changed 10 years ago by gigem

(In [21927]) Change the schedul editor to use the regular recordid if it exists when previewing changes. stuartm, change this if you don't like it.

Refs #7080.

comment:10 Changed 10 years ago by stuartm

(In [21933]) Fix the bug that I think [21927] was addressing. We assign m_recordID to m_tempID when m_recordID > 0, but then we trampled m_tempID in the same way as the recordid in [21923]. Reverts [21927] since that change is redundant with the proper fix in place.Refs #7080

comment:11 Changed 10 years ago by gigem

Resolution: fixed
Status: newclosed

This is now fixed with the recent commits.

Note: See TracTickets for help on using tickets.