Update dependencies and add Github Actions #26

phillipthelen · 2021-09-01T12:30:30Z

this is still WIP. As a test I added a GH action to lint the project.

SabreCat · 2021-09-10T20:55:21Z

@phillipthelen This looks fantastic! Great job adding all those Actions checks. Might there be a way for us to run this through a staging test? Put it up on a freebie Heroku or Hetzner server and point Staging at it for a little while, see if anything crops up?

phillipthelen · 2021-09-13T08:21:30Z

I don't have experience setting up our projects on heroku, but it should be possible, yes. Pointing staging to it sounds good.

libs/mobilePayments.js

libs/applePayments.js

SabreCat · 2022-01-05T17:08:21Z

libs/googlePayments.js

-        let purchaseDataList = iap.getPurchaseData(response);
-        let subscription = purchaseDataList[0];
+        const purchaseDataList = iap.getPurchaseData(response);
+        const subscription = purchaseDataList[0];
        if (subscription.expirationDate < jobStartDate) {


ha, the logic is the same but the conditions are inverted, vs Apple... what :D

The logic was also inverted. the if case being cancellation here and another check on iOS. I flipped it here, so that both use the same check/logic for consistency

SabreCat

Did a much closer look--probably nitpicky, but while we're in here cleaning house, might be good?

SabreCat · 2022-01-05T17:11:55Z

libs/googlePayments.js

  return habitrpgUsers.find(query, {
-    sort: {_id: 1},
+    sort: { _id: 1 },
    limit: USERS_BATCH,
    fields: ['_id', 'apiToken', 'purchased.plan'],


This is the same lookup as on Apple, but it doesn't do the blocked user check--

Should it?

Does this suggest we should abstract the find function to a lib?

I removed the block check from Apple. Thinking about it, we don't really want that. If an account is blocked, we'd still want to (properly) cancel their subscription. Otherwise it will get checked over and over.

Moving more code into a lib to avoid code duplication sounds smart. But I would do that in a separate PR.

SabreCat · 2022-01-05T17:14:25Z

libs/groupSubscriptionManager.js

@@ -13,17 +13,6 @@ var pageLimit = 10;

 var SUBSCRIPTION_CANCEL_URL = 'https://habitica.com/amazon/subscribe/cancel';


Can we get the BASE_URL from the environment and append the rest of the path?

SabreCat · 2022-01-05T17:20:58Z

libs/pushNotifications.js

-var gcmLib = require('node-gcm'); // works with FCM notifications too
-var AWS = require('aws-sdk');
+import nconf from 'nconf';
+/*import pushNotify from 'push-notify';


Big commented-out file? If we're not handling this here anymore, can we delete it proper-like?

SabreCat · 2022-01-05T17:32:04Z

libs/subscriptionsReminders/amazon.js

@@ -18,12 +15,12 @@ api.scheduleNextCheckForUser = function scheduleNextCheckForUser (habitrpgUsers,
      $set: {
        'purchased.plan.lastReminderDate': moment().toDate(),
      },
-    }
+    },
  );


Do we need to .exec() this?

or, wait, we're using monk rather than mongoose, is that why?

SabreCat · 2022-01-05T17:34:40Z

libs/subscriptionsReminders/amazon.js

@@ -84,18 +72,18 @@ api.processUser = function processUser (habitrpgUsers, user, queue, baseUrl, job
    'Found user with id', user._id, 'lastReminderDate', user.purchased.plan.lastReminderDate,
    'last paymentdate', lastBillingDate.toString(),
    'next date', nextBillingDate.toString());
-  console.log('Plan', plan);*/
+  console.log('Plan', plan); */


Various commented out console logs--do we need these?

SabreCat · 2022-01-05T17:37:54Z

libs/subscriptionsReminders/amazonGroups.js

@@ -18,14 +15,14 @@ api.scheduleNextCheckForGroup = function scheduleNextCheckForGroup (habitrpgGrou
      $set: {
        'purchased.plan.lastReminderDate': moment().toDate(),
      },
-    }
+    },
  );


same things in this file as Amazon reminders base--exec? console logs?

SabreCat · 2022-01-05T21:33:06Z

server.js

+  });
+  queue.on('failed', (job, error) => {
+    console.log(error);
+    /* const args = Array.prototype.slice.call(arguments);


More commented out code? I understand on the recap emails, it's a good reminder that such a thing exists and could be enabled again, but what about this?

SabreCat · 2022-01-05T21:37:18Z

workers/email.js

+import fs from 'fs';
+import nconf from 'nconf';
+import mandrill from 'mandrill-api';
+import _ from 'lodash';


Looks like we only use find and findIndex here, can we import those individually?

SabreCat · 2022-01-05T21:38:52Z

workers/onboardingEmails.js

-const emailsLib = require('../libs/email');
-const USERS_BATCH = 10;
+import moment from 'moment';
+import lodash from 'lodash';


random, template, and assign--
if nothing else, we should probably be consistent about importing it as _ vs lodash!

phillipthelen added 10 commits September 1, 2021 14:26

Update packages and code to ES6

2fa638d

add linting action

8cd5dbd

use correct command

23702c4

Update dependencies and replace kue with bull

5007468

Make server run correctly

7252f6f

Correctly return server health

87004b2

Fix lint errors

f2b8f56

fix group subscription manager

0a756ac

add first try at running tests on GH

447e1e9

rename example config file

79d613c

phillipthelen requested a review from SabreCat September 8, 2021 13:02

SabreCat reviewed Jan 5, 2022

View reviewed changes

libs/mobilePayments.js Outdated Show resolved Hide resolved

SabreCat reviewed Jan 5, 2022

View reviewed changes

libs/applePayments.js Outdated Show resolved Hide resolved

SabreCat reviewed Jan 5, 2022

View reviewed changes

SabreCat requested changes Jan 5, 2022

View reviewed changes

SabreCat and others added 7 commits January 5, 2022 15:44

fix(typos): this commit does not exists

582d63e

Address commentts

617ef43

Merge branch 'master' into actions

0355de4

Don’t fail worker when a single user errors

1d7e6db

various refactorings

2b4d0f2

Send notifications on failed jobs

65f6ce2

refactor to remove duplicate code

50fdf95

phillipthelen force-pushed the actions branch from 1d745de to 50fdf95 Compare October 14, 2022 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies and add Github Actions #26

Update dependencies and add Github Actions #26

phillipthelen commented Sep 1, 2021

SabreCat commented Sep 10, 2021

phillipthelen commented Sep 13, 2021

SabreCat Jan 5, 2022

phillipthelen Aug 15, 2022

SabreCat left a comment

SabreCat Jan 5, 2022

phillipthelen Aug 15, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

SabreCat Jan 5, 2022

		@@ -13,17 +13,6 @@ var pageLimit = 10;

		var SUBSCRIPTION_CANCEL_URL = 'https://habitica.com/amazon/subscribe/cancel';

Update dependencies and add Github Actions #26

Are you sure you want to change the base?

Update dependencies and add Github Actions #26

Conversation

phillipthelen commented Sep 1, 2021

SabreCat commented Sep 10, 2021

phillipthelen commented Sep 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SabreCat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment