Rewrite emailService.js to emailService.ts #985

niccolopaganini · 2023-09-22T22:54:24Z

Child Issue of #977 (Angular Services needing rewrite)

Addressing this file: https://github.com/e-mission/e-mission-phone/blob/fce117ff859abd995613bd405dbc7d27c703b09b/www/js/control/emailService.js

I will work on converting this file from emailService.js to emailService.ts

The text was updated successfully, but these errors were encountered:

shankari · 2023-09-22T23:02:21Z

@niccolopaganini a couple of high level things to watch out for:

This uses angular services such as $window and $http. You will need to look at the prior migrations or the existing ts files to see how they have been handled (e.g. https://github.com/search?q=repo%3Ae-mission%2Fe-mission-phone+window&type=pullrequests)
It also uses the Logger angular service, which I believe has already been rewritten into tsx, but will need to be imported "the react way" (e.g. https://github.com/search?q=repo%3Ae-mission%2Fe-mission-phone+Logger&type=pullrequests)

niccolopaganini · 2023-10-12T18:57:01Z

Couple of things that I noticed while changing the script

Handling 'use strict'; -> https://stackoverflow.com/questions/31391760/use-strict-needed-in-a-typescript-file -> seems like including them is useful and requires less code maintenance in the future
the Js file depends on 'json/emailConfig.json' -> I looked at the directories at a high level but I seem to only locate emailConfig.json.sample; not sure what I am missing (From my understanding, the sample JSON file seems to be like a backup) @JGreenlee, how should I handle this (code chunk below for reference).

const getEmailConfig = function () {
            return new Promise(function (resolve, reject) {
                window.Logger.log(window.Logger.LEVEL_INFO, "About to get email config");
                var address = [];
                $http.get("json/emailConfig.json").then(function (emailConfig) {
                    window.Logger.log(window.Logger.LEVEL_DEBUG, "emailConfigString = " + JSON.stringify(emailConfig.data));
                    address.push(emailConfig.data.address)
                    resolve(address);
                }).catch(function (err) {
                    $http.get("json/emailConfig.json.sample").then(function (emailConfig) {
                        window.Logger.log(window.Logger.LEVEL_DEBUG, "default emailConfigString = " + JSON.stringify(emailConfig.data));
                        address.push(emailConfig.data.address)
                        resolve(address);
                    }).catch(function (err) {
                        window.Logger.log(window.Logger.LEVEL_ERROR, "Error while reading default email config" + err);
                        reject(err);
                    });
                });
            });
        }

Current status: trying to wrap my head around www/js/i18nextInit.ts (not necessarily required?) but thought this would be a good opportunity to do so

JGreenlee · 2023-10-12T19:28:43Z

You don't need 'use strict';. None of the other rewrites have included 'use strict';.

The reason is that you are creating a module (which just means it exports something that can be imported by other files). Modules are always in strict mode anyway https://stackoverflow.com/a/31685340.

JGreenlee · 2023-10-12T19:46:11Z

I think we are probably going to reconsider emailConfig.json anyway and come up with a different strategy for email configuration.
For now, I would just replace all of that code and put k.shankari@nrel.gov directly in the code (I am assuming Shankari will prefer this over the old Berkeley email)

@shankari how should we approach this moving forward? Should we start supporting another optional field in the dynamic config for this?
And if it's not supplied, we should fallback to k.shankari@nrel.gov?

niccolopaganini · 2023-10-18T19:27:54Z

running npm run serve gave me this:

ERROR in ./www/index.js 27:0-38
Module not found: Error: Can't resolve './js/control/emailService.js' in '/Users/nseptank/e-mission-phone-nvsr-iter1/www'
resolve './js/control/emailService.js' in '/Users/nseptank/e-mission-phone-nvsr-iter1/www'
  using description file: /Users/nseptank/e-mission-phone-nvsr-iter1/package.json (relative path: ./www)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: /Users/nseptank/e-mission-phone-nvsr-iter1/package.json (relative path: ./www/js/control/emailService.js)
      no extension
        Field 'browser' doesn't contain a valid alias configuration
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js doesn't exist
      .web.js
        Field 'browser' doesn't contain a valid alias configuration
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js.web.js doesn't exist
      .jsx
        Field 'browser' doesn't contain a valid alias configuration
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js.jsx doesn't exist
      .tsx
        Field 'browser' doesn't contain a valid alias configuration
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js.tsx doesn't exist
      .ts
        Field 'browser' doesn't contain a valid alias configuration
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js.ts doesn't exist
      .js
        Field 'browser' doesn't contain a valid alias configuration
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js.js doesn't exist
      as directory
        /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.js doesn't exist

Changing package.json and going to see the changes...

niccolopaganini · 2023-10-18T20:09:28Z

Clearly package.json seems to be the wrong place to look. I peeked into a couple of files and it seems that it is being called at index.js.

Underneath that file, I change the file name emailService.js to emailService.tsx

niccolopaganini · 2023-10-18T23:25:42Z

I activated it and ran the command to get the server up and running to test but got this:

ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx
./www/js/control/emailService.tsx 30:33-40
[tsl] ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx(30,34)
      TS1005: ';' expected.
ts-loader-default_8025a27c15fa23e2
 @ ./www/index.js 27:0-39

ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx
./www/js/control/emailService.tsx 88:33-40
[tsl] ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx(88,34)
      TS1005: ';' expected.
ts-loader-default_8025a27c15fa23e2
 @ ./www/index.js 27:0-39

webpack 5.89.0 compiled with 2 errors and 6 warnings in 13007 ms

Ignoring the warnings (at least for now), it indicated that there were two semicolons missing, but going through the code, I cannot find anything. What I can notice is some inconsistency with curly-braces. Going through the code now...

niccolopaganini · 2023-10-18T23:53:19Z

Figured... I missed one }; and that threw this error. I am confident this will fix 🤞

niccolopaganini · 2023-10-19T01:04:59Z

Okay it did not, the bracket was part of the problem but not the whole solution. Let me stare at it a little bit and see what's up.

shankari · 2023-10-19T02:27:45Z

I think we are probably going to reconsider emailConfig.json anyway and come up with a different strategy for email configuration.
For now, I would just replace all of that code and put k.shankari@nrel.gov directly in the code (I am assuming Shankari will prefer this over the old Berkeley email)

Yes, this is good.

@shankari how should we approach this moving forward? Should we start supporting another optional field in the dynamic config for this?
And if it's not supplied, we should fallback to k.shankari@nrel.gov?

I think that there should be two configs.

One app-level config which is changed while creating a custom app, which controls:
- the URL prefix (emission:// versus nrelopenpath://),
- emailConfig,
- uploadConfig (⬆️ ☁️ Upload Service -- rewrite and testing e-mission-phone#1053 (comment)) etc.
- It could also have a flag indicating whether we want it to be a multi-tenant, dynamically configurable app, or a single user app. This can even be filled in statically using webpack instead of being read at runtime.
- And if it is a dynamic app, then what is the URL to download data from?
One dynamic config, which allows a single app to be configured based on partner preferences.

I wanted to implement this #850 (comment), #850 (comment)
but ran out of time.

Once we implement this app-level config, we can close #850

niccolopaganini · 2023-10-19T18:55:38Z

I tried peeking into the code but I am unable to find anything. Considering this code chunk:

const getEmailConfig = function () {
...

is going to be replaced anyway, I went forward with changing the code and then troubleshoot. Please let me know if this is a good approach @JGreenlee. Since you're traveling, I will insert the code chunk here for convenience (hopefully 🥶):

const getEmailConfig = function (): Promise<string[]> {
            return new Promise(function (resolve, reject) {
              window.Logger.log(window.Logger.LEVEL_INFO, "About to get email config");
              setTimeout(() => {
                const emailConfig = "k.shankari@nrel.gov"; // email address
          
                window.Logger.log(window.Logger.LEVEL_DEBUG, "emailConfigString = " + emailConfig);
                resolve([emailConfig]);
              }, 1000);
            });
          };

niccolopaganini · 2023-10-19T20:18:59Z

Quick Question:

this.sendEmail = function (database) {
            Promise.all([getEmailConfig(), hasAccount()]).then(function([address, hasAct]) {
                var parentDir = "unknown";

                if (ionic.Platform.isIOS() && !hasAct) {
                    alert(i18next.t('email-service.email-account-not-configured'));
                    return;
                }

                if (ionic.Platform.isAndroid()) {
                    parentDir = "app://databases";
                }
                if (ionic.Platform.isIOS()) {
                    alert(i18next.t('email-service.email-account-mail-app'));
                    parentDir = cordova.file.dataDirectory + "../LocalDatabase";
                }

                if (parentDir == "unknown") {
                    alert("parentDir unexpectedly = " + parentDir + "!")
                }

                window.Logger.log(window.Logger.LEVEL_INFO, "Going to email " + database);
                parentDir = parentDir + "/" + database;
                /*
                window.Logger.log(window.Logger.LEVEL_INFO,
                    "Going to export logs to "+parentDir);
                 */
                alert(i18next.t('email-service.going-to-email', { parentDir: parentDir }));
                var email = {
                    to: address,
                    attachments: [
                        parentDir
                    ],
                    subject: i18next.t('email-service.email-log.subject-logs'),
                    body: i18next.t('email-service.email-log.body-please-fill-in-what-is-wrong')
                }

                window['cordova']plugins.email.open(email, function () {
                  Logger.log("email app closed while sending, "+JSON.stringify(email)+" not sure if we should do anything");
                  // alert(i18next.t('email-service.no-email-address-configured') + err);
                  return;

Since we're going to replace with Shankari's email address, can I rewrite the above it in such a way that the "function" remains the same? Something like:

var email = {
to: 
sub: 
...

What do you think @JGreenlee ?

niccolopaganini · 2023-10-20T19:01:26Z

For the current changes that I have rewritten, these are the errors I am getting:

ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx
./www/js/control/emailService.tsx 5:4-5
[tsl] ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx(5,5)
      TS1128: Declaration or statement expected.
ts-loader-default_8025a27c15fa23e2
 @ ./www/index.js 27:0-39

ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx
./www/js/control/emailService.tsx 66:33-40
[tsl] ERROR in /Users/nseptank/e-mission-phone-nvsr-iter1/www/js/control/emailService.tsx(66,34)
      TS1005: ';' expected.
ts-loader-default_8025a27c15fa23e2
 @ ./www/index.js 27:0-39

niccolopaganini · 2023-10-20T19:19:22Z

I kind of rewrote the code from what I can understand .ts and I am facing WSOD for now. Let me look into the code and what I broke.

niccolopaganini · 2023-10-20T19:19:43Z

I kind of rewrote the code to address the issues but let's see...

niccolopaganini · 2023-10-20T19:26:51Z

Peeked into the CLI when running the server and this is what I got:

[0] [phonegap] [console.warn] "shadow*" style props are deprecated. Use "boxShadow".
[0] [phonegap] [console.warn] Missing translation for key 'survey-missing-formpath'
[0] [phonegap] [console.warn] Missing translation for key 'profile-tab'
[0] [phonegap] [console.warn] Missing translation for key 'incorrect-app-status'
[0] [phonegap] [console.warn] Missing translation for key 'fix-app-status'
[0] [phonegap] [console.warn] Missing translation for key 'fix'
[0] [phonegap] [console.warn] Missing translation for key 'log-title'
[0] [phonegap] [console.warn] Missing translation for key 'sensed-title'
[0] [phonegap] [console.warn] Missing translation for key 'reminders-time-of-day'
[0] [phonegap] [console.warn] Missing translation for key 'upcoming-notifications'
[0] [phonegap] [console.warn] Missing translation for key 'dummy-notification'
[0] [phonegap] [console.warn] Missing translation for key 'log-out'
[0] [phonegap] [console.warn] Missing translation for key 'are-you-sure'
[0] [phonegap] [console.warn] Missing translation for key 'log-out-warning'
[0] [phonegap] [console.warn] Missing translation for key 'confirm'
...

niccolopaganini · 2023-10-20T19:27:10Z

I am not sure where I got it wrong. Reverting the changes I made and restoring the original code structure

Abby-Wheelis · 2023-10-20T21:36:09Z

Most of the other service rewrites have been from .js -> .ts files, not necessarily .tsx files. Glancing at emailService.js, I don't see any visual elements, so I think you'd be good to go to a .ts file rather than a .tsx file.

I have also gotten WSOD a few times when I've been re-writing, and it's typically been that I forgot to remove/replace all references to the old version of the file. When you get WSOD, it can help to have the debugger connected and see what the error message is (it's usually super long, but normally points me in the right direction).

[0] [phonegap] [console.warn] Missing translation for key 'survey-missing-formpath'
[0] [phonegap] [console.warn] Missing translation for key 'profile-tab'
[0] [phonegap] [console.warn] Missing translation for key 'incorrect-app-status'

Warnings like these are usually not the problem, even if a translation is missung the code usually still runs.

Long term, this should be part of the app config e-mission/e-mission-docs#985 (comment)

niccolopaganini · 2023-10-25T20:18:28Z

Took a lot of advice from @Abby-Wheelis. I currently rewrote the entire thing just to see if it works (@JGreenlee suggested to just use Shankari's email address). I removed import './js/control/emailService.js'; from index.js. I also removed emission.services.email from ngApp.js. I am trying to maintain the same stylistic language that Jiji suggested for uniformity.

I am unable to notice any errors in the inspect console in Google Chrome but as soon as I try to navigate to the Profile section in the DevApp, it shows a WSOD. I currently have no idea why...

Abby-Wheelis · 2023-10-25T20:26:06Z

ProfileSettings calls getAngularService for EmailHelper (other file may as well, but I know this one does) one potential cause for WSOD here is that call is still in place, and it can't find the service, so it fails on startup.

niccolopaganini · 2023-12-03T19:52:22Z

Closing this issue since its resolved

JGreenlee mentioned this issue Oct 16, 2023

⬆️ ☁️ Upload Service -- rewrite and testing e-mission/e-mission-phone#1053

Merged

shankari mentioned this issue Oct 25, 2023

💪 Increase checks for scanned QR code e-mission/e-mission-phone#1081

Merged

shankari added a commit to Abby-Wheelis/e-mission-phone that referenced this issue Oct 25, 2023

Use only emission externally

2d930f8

Long term, this should be part of the app config e-mission/e-mission-docs#985 (comment)

niccolopaganini closed this as completed Dec 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite emailService.js to emailService.ts #985

Rewrite emailService.js to emailService.ts #985

niccolopaganini commented Sep 22, 2023

shankari commented Sep 22, 2023

niccolopaganini commented Oct 12, 2023

JGreenlee commented Oct 12, 2023

JGreenlee commented Oct 12, 2023

niccolopaganini commented Oct 18, 2023 •

edited

Loading

niccolopaganini commented Oct 18, 2023

niccolopaganini commented Oct 18, 2023

niccolopaganini commented Oct 18, 2023 •

edited

Loading

niccolopaganini commented Oct 19, 2023

shankari commented Oct 19, 2023 •

edited

Loading

niccolopaganini commented Oct 19, 2023 •

edited

Loading

niccolopaganini commented Oct 19, 2023 •

edited

Loading

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023 •

edited

Loading

Abby-Wheelis commented Oct 20, 2023 •

edited

Loading

niccolopaganini commented Oct 25, 2023

Abby-Wheelis commented Oct 25, 2023

niccolopaganini commented Dec 3, 2023

Rewrite emailService.js to emailService.ts #985

Rewrite emailService.js to emailService.ts #985

Comments

niccolopaganini commented Sep 22, 2023

Child Issue of #977 (Angular Services needing rewrite)

shankari commented Sep 22, 2023

niccolopaganini commented Oct 12, 2023

JGreenlee commented Oct 12, 2023

JGreenlee commented Oct 12, 2023

niccolopaganini commented Oct 18, 2023 • edited Loading

niccolopaganini commented Oct 18, 2023

niccolopaganini commented Oct 18, 2023

niccolopaganini commented Oct 18, 2023 • edited Loading

niccolopaganini commented Oct 19, 2023

shankari commented Oct 19, 2023 • edited Loading

niccolopaganini commented Oct 19, 2023 • edited Loading

niccolopaganini commented Oct 19, 2023 • edited Loading

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023

niccolopaganini commented Oct 20, 2023 • edited Loading

Abby-Wheelis commented Oct 20, 2023 • edited Loading

niccolopaganini commented Oct 25, 2023

Abby-Wheelis commented Oct 25, 2023

niccolopaganini commented Dec 3, 2023

niccolopaganini commented Oct 18, 2023 •

edited

Loading

niccolopaganini commented Oct 18, 2023 •

edited

Loading

shankari commented Oct 19, 2023 •

edited

Loading

niccolopaganini commented Oct 19, 2023 •

edited

Loading

niccolopaganini commented Oct 19, 2023 •

edited

Loading

niccolopaganini commented Oct 20, 2023 •

edited

Loading

Abby-Wheelis commented Oct 20, 2023 •

edited

Loading