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

Fixes for #14 and #15 #30

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fixes for #14 and #15 #30

wants to merge 5 commits into from

Conversation

spacezorro
Copy link

So removing the sorting of the ruleset (a39fd64) allows for the rules to be output in yaml file order.

Adding a delete all rules (f1652b7) and then re-uploading the rules with a delay (115c5aa) has all the rules in Gmail in the same order as the yaml file.

I also changed the OATH2 to token response (eeefdab) because the webserver wasn't working and token always works.

I also added a smartlabel alternative (6ab9bed) for the "Categories"

Erin Quinlan added 5 commits September 14, 2021 16:24
adds "smartlabel"
e.x.
  smartlabel: ^smartlabel_promo

This allows you to assign a label and a category to the email in the same rule

Smartlabels:
Updates = ^smartlabel_notification
Primary = ^smartlabel_personal
Forum = ^smartlabel_group
Social = ^smartlabel_social
Promotions = ^smartlabel_promo
…efore generating the rules

It generates the rules in the same order as the yaml file.
Copy link
Owner

@mesozoic mesozoic left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request and the bug reports! I'm sorry for the (super) delayed response, but I've somehow managed to totally mangle my Github notifications and I've yet to take the time to fix it.

Since there are a few different changes in this branch it's hard for me to merge; I'd prefer to do each pull request separately so that each change in behavior is atomic. I'll leave comments inline where they seem most relevant.

@@ -198,6 +199,7 @@ def upload_ruleset(ruleset, service=None, dry_run=False):
request = service.users().settings().filters().create(userId='me', body=filter_data)
if not dry_run:
request.execute()
time.sleep(1.5)
Copy link
Owner

Choose a reason for hiding this comment

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

I'm uncomfortable injecting any amount of delay here. You could make this configurable, I suppose, but I'm still not sure I see why this is necessary. Can you clarify how it helps?

Copy link
Author

Choose a reason for hiding this comment

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

When you are uploading/sending recipes into gmail and you want them to be in a specific order there needs to be a cooldown between sending messages. Gmail seems to process the recipe then add it to the list. If there is no delay then gmail could take a list of recipes designed to go 12345 and add it as 12435

It's only needed when you are sending the ruleset unsorted because you have rules designed on previous rules

I found ~1.5sec was the best

@@ -178,6 +178,7 @@ class RuleCondition(_RuleConstruction):
'does_not_have': 'doesNotHaveTheWord',
'missing': 'doesNotHaveTheWord',
'no_match': 'doesNotHaveTheWord',
'smartlabel': 'smartLabelToApply',
Copy link
Owner

Choose a reason for hiding this comment

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

The smartlabel/category issue came up in #33, too; I suspect that their XML import and their API are subtly incompatible in this one regard, and we'll need a way to handle both of those situations differently based on which command the user is running. Let's move discussion of smartlabels there.

Copy link
Author

@spacezorro spacezorro Mar 30, 2022

Choose a reason for hiding this comment

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

Posting this here but I will also follow up in #33

I wanted to be able to do something like the following

  • from: "mom.at.email"
    label: Family
    smartlabel: ^smartlabel_personal

I found the following smartlabels match with the following categories
category:primary ^smartlabel_personal
category:social ^smartlabel_social
category:promotions ^smartlabel_promo
category:updates ^smartlabel_notification
category:forums ^smartlabel_group
category:reservations
category:purchases
category:travel

The "^" on a smartlabel is required for the XML import to work

When you are matching you have to use "category: primary" but if you want to set the category you use "smartlabel: ^smartlabel_personal"

Copy link
Author

Choose a reason for hiding this comment

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

I think this is leftover Inbox stuff

Comment on lines -601 to +602
for rule in sorted(ruleset):
for rule in ruleset:
Copy link
Owner

Choose a reason for hiding this comment

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

I'm open to removing the sorting. I am scratching my head trying to remember why I did that in the first place, but I can't recall. Might've had something to do with removing duplicates? 🤷 I'd love help coming up with a test for #14 so we can ensure this doesn't regress at some point in the future.

Comment on lines -278 to +282
credentials = oauth2client.tools.run_flow(flow, store, flags=flags_parser.parse_args([]))
flags=flags_parser.parse_args([])
flags.noauth_local_webserver = True
credentials = oauth2client.tools.run_flow(flow, store, flags)
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure I understand what this fixes, though I believe you that it helped somewhere. Do you have steps to reproduce an error?

Copy link
Author

Choose a reason for hiding this comment

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

I was doing this from a completely headless system, so I can't start up chrome or whatever to do all the oath.

This allows all the oath to happen on the command line.

Comment on lines -102 to +107
if args.action == 'upload':
if args.action == 'delete':
emptyruleset={}
prune_filters_not_in_ruleset(emptyruleset, service=gmail, dry_run=args.dry_run)
elif args.action == 'upload':
Copy link
Owner

Choose a reason for hiding this comment

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

The --delete command is a bit scary without any backup, but since we already have --prune I don't think it's any scarier than status quo. 😁 I'd merge that if it were a separate branch. I don't have a great way to write a regression test for that, though.

Copy link
Owner

Choose a reason for hiding this comment

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

I've implemented this as --delete-all in 9454797; it'll be part of the next release.

Copy link
Author

Choose a reason for hiding this comment

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

This is more of my "rules in order" stuff. (#14)

Since you can't insert a changed rule you have to delete all the rules and re-add them to get them in order.

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