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

EPG - Delete timer 'All' option fails to delete timer rule #15410

Merged
merged 1 commit into from
Feb 3, 2019
Merged

EPG - Delete timer 'All' option fails to delete timer rule #15410

merged 1 commit into from
Feb 3, 2019

Conversation

linknetx
Copy link
Contributor

@linknetx linknetx commented Feb 3, 2019

Description

In the EPG, selecting 'Delete timer' on a program with a series timer rule opens the 'Confirm delete' dialog asking 'Do you only want to delete this timer or also the timer rule that has scheduled it?' with two options 'All' or 'Only this'.

Both options have the same result and only delete the selected timer. The 'All' option fails to delete the timer rule.

This PR fixes this issue with the 'All' option now deleting the parent timer rule.

In the original code 'ruleTag' is never returned, only 'Tag' is returned for both options.

Motivation and Context

As above.

How Has This Been Tested?

Compiled and tested 'All' and 'Only this' options on an EPG program with series timer rule. The 'All' option now deletes the parent timer rule as expected.

Screenshots (if appropriate):

See comments for screenshot of the dialog referenced above

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@Rechi Rechi requested a review from ksooo February 3, 2019 08:35
Fix for this issue:

In the EPG, selecting 'Delete timer' on a program with a series timer rule opens the 'Confirm delete' dialog asking 'Do you only want to delete this timer or also the timer rule that has scheduled it?' with two options 'All' or 'Only this'.

Both options have the same result and only delete the selected timer. The 'All' option fails to delete the timer rule.
@linknetx
Copy link
Contributor Author

linknetx commented Feb 3, 2019

Referenced 'Confirm delete' dialog:

confirm-delete-dialog

@linknetx
Copy link
Contributor Author

linknetx commented Feb 3, 2019

Just checked, this line of code was present in 17.6 but has been omitted in 18.0

Copy link
Member

@ksooo ksooo left a comment

Choose a reason for hiding this comment

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

Good catch. Thanks.

@ksooo ksooo merged commit 077fde4 into xbmc:master Feb 3, 2019
@ksooo ksooo added Type: Fix non-breaking change which fixes an issue Component: PVR 18.1 labels Feb 3, 2019
@ksooo ksooo added this to the Leia 18.1-rc1 milestone Feb 3, 2019
@ksooo
Copy link
Member

ksooo commented Feb 3, 2019

@linknetx and what I overlooked in the first place - next time please use more meaningful commit messages, thanks.

@MartijnKaijser
Copy link
Member

@ksooo at least the commit message has extra text on the extra lines

@linknetx
Copy link
Contributor Author

linknetx commented Feb 3, 2019

@ksooo what would be a more meaningful commit message to describe this?

@Rechi Rechi added the v18 Leia label Feb 3, 2019
@ksooo
Copy link
Member

ksooo commented Feb 3, 2019

Ah, I missed the extra lines. ;-) In github web ui you just see

screenshot 2019-02-03 at 17 40 00

... which is not very meaningful...

@ksooo
Copy link
Member

ksooo commented Feb 3, 2019

We usually do not include "Update foo.bar" in our commit messages (and in no case as first line).

@linknetx
Copy link
Contributor Author

linknetx commented Feb 3, 2019

Fair comment, I see the problem, I was being lazy and used the GitHub Desktop default summary which I usually edit into something more meaningful.

@linknetx linknetx deleted the delete-timer branch February 4, 2019 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants