Skip to content

feat: add duplicate cron job functionality#58

Open
simon-suess wants to merge 1 commit into
antoniorodr:mainfrom
simon-suess:duplicate-cronjob
Open

feat: add duplicate cron job functionality#58
simon-suess wants to merge 1 commit into
antoniorodr:mainfrom
simon-suess:duplicate-cronjob

Conversation

@simon-suess

Copy link
Copy Markdown
Contributor

What this changes

Duplicate cron jobs with d keybinding. Copies:

  • Job ID with _copy suffix (e.g., backup_jobbackup_job_copy)
  • Schedule expression and command
  • Log settings (enable/disable logging status)
  • Works for local and remote SSH jobs

Updates:

  • CronTable: action_duplicate_cronjob() method
  • CronCreator: accepts optional log_enabled parameter
  • app.py: passes duplicate_data dict to editor
  • README: added feature to list + keyboard shortcuts table
  • Extracted NO_ID_LABEL constant (DRY)

How I tested this

  • All 108 unit tests pass
  • Manual verification: feature works on my machine xD
  • Added 4 binding/method tests in CronTable_duplicate_test.py

Checklist

  • I have read CONTRIBUTING.md and this PR follows the guidelines
  • A human has reviewed the entire diff of this PR, every line of code
  • A human understands the changes and can explain why this approach is correct
  • This PR doesn't have AI-generated boilerplate or co-author lines
  • This PR was authored and submitted by an AI agent without human review

@antoniorodr

Copy link
Copy Markdown
Owner

Hello @simon-suess!

Thanks for this PR.

A question: In which scenarios will be this useful? I mean, why would someone want to duplicate a cronjob?

I am not saying this is not useful, I just want to understand the use case better.

Thanks!

@simon-suess

Copy link
Copy Markdown
Contributor Author

Hello @antoniorodr,

 if you think that this feature is not needed or some people wouldn't or most people wouldn't use it, feel free to keep it out. I mainly did it for myself, but thought that contributing would be cool if other people needed it too.
 
 For me it felt natural to also have a duplicate feature and I just needed it. I use Cornboard on my Mac to do updates, do some networking stuff. And for example I duplicated my old cron job where everything is configured and just changed the script that is executed. So everything else is already configured, i just change the command. Another time i used it for example, I want to update my brew dependencies once at 8am in the morning. But if I am not yet logged or have my Mac open it wont do it, so i duplicated it and set the start time to 9am. So even if I start work a bit later, I still update brew.
 
 Hope that makes my use case clear. Thanks.

@antoniorodr

Copy link
Copy Markdown
Owner

@simon-suess

I will check the PR out. I understand the use case you are trying to address, and it does not seem like a bad idea to me. And it does not hurt to habe more options for users.

I am feeling under the weather tho, but I will try to review it as soon as I can. Thanks for your patience!

@antoniorodr

Copy link
Copy Markdown
Owner

Hello @simon-suess!

I have not forgotten about this PR, but I have been busy with other things. I will try to review it this week. Thanks for your patience!

@antoniorodr

Copy link
Copy Markdown
Owner

Hello @simon-suess!

I have not forgotten about this PR, but I have been busy with other things. I will try to review it this week. Thanks for your patience!

@antoniorodr antoniorodr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I have added some comments to the PR. I have not checked all the files, because I prefer you to answer/fix this first.

Thanks for your patience!

Comment thread .gitignore
update_dependencies.py
test-server/

.idea

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
.idea
.idea

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What I meant was: why did you add this to the file?

Comment thread README.md
The default starting point for autocompletion is the home directory of the user whose cron jobs you are managing. Accept a suggestion with the `Tab` key.

### 📋 Keyboard Shortcuts

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

They shortcurs are covered on the website documentation. That's why I didn't choose to have it on the README.md.

identificator = row[0]
expr = row[1]
cmd = row[2]
log_enabled = row[3] == "True" # Convert string to bool

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why are you converting a bool to string? And why are you converting it this way?

self.identificator = identificator
self.log_enabled = has_wrapper(command) if command else False
# If log_enabled is explicitly provided, use it; otherwise detect from command
self.log_enabled = log_enabled if log_enabled is not None else (has_wrapper(command) if command else False)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Here are you using self.log_enabled as a boolean, but on CronTable you are converting the value to a string.

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.

2 participants