Upgrade pg and sequelize dependencies

[jbeard4]

Tested on node 18.6.0

[Rovack]

IIRC you can specify this once in a jest config file rather than in every file.

Though actually, their docs claim "node" is the default anyway, so how come it needs to be specified at all?
(Could be the docs are for a newer version, I suppose.)

[jbeard4]

Not sure why this was needed, but it was needed in order to get the tests running.

[Rovack]

You mean specifying it once in the jest config file also doesn't work?

[Rovack]

So I see there's been a lot of discussion about v5's breaking change around using camelCase for the JS attributes even when underscored is true. Sounds like it is possible to work around it, quite elegantly for timestamps but only on a model-by-model basis for foreign keys.

Still, I wonder if it wouldn't be preferable to do that, given the not-huge number of models in this repo. I assume that's the only thing preventing us from just using the same Lambda for YP and GG.
It probably doesn't matter so much for GG itself, but assuming there'll be work around the SSTT too (which is ultimately the same app), a shared BE with YP could make things much simpler.

[jbeard4]

Hi @Rovack,

Thanks for posting these links. I read everything, and I it seems clear that sequelize is not trying to maintain backward compatibility between v4 and v5/6. Specifically, I would expect property names to change in two ways:

1. {created,updated}At from snake case to camel case, and
2. Foreign keys will be named differently unless overridden.

For {created,updated}At, I did some experimenting to try the following workarounds, but they did not appear to work in v6:

1. Using underscored: true still returns attributes in camelCase sequelize/sequelize#10857 (comment)
2. Several bugs/questions regarding underscored/underscoredAll in Sequelize v5 sequelize/sequelize#11225 (comment)

So I think that, for these properties, we should update references to them in the GG UI, because I do not have a workaround. There do not appear to be that many references to these properties in the GG UI, so should be pretty doable.

For foreign keys, the default property names (e.g. location.OrganizationId) can be overridden like this:

Location.belongsTo(models.Organization, { foreignKey: 'organization_id' });

Then you get location.organization_id, as expected. So I will try to update this for all the FK relationships in all of the model definitions.

[jbeard4]

I think the main reason to run a separate lambda is because:

1. There is not a straightforward way to ensure that sequelize after the version upgrade will generate the same runtime data structures that it was generating before the version upgrade, and
2. Changes to runtime data structures could break GG
3. I don't want to break GG

Maybe the solution is to test GG thoroughly with the service after the version upgrade. As I mentioned in the comment above, I think at least the references to the {created,updated}At properties in the UI code will need to be updated to accommodate the new version of sequelize, as I have not found a workaround for making these properties snake case.

I'm going to work on getting GG running locally and see if i can get the whole thing running end-to-end.

[jbeard4]

@Rovack

Everything is now running end-to-end on my local. I spent about 10m clicking around and exercising things, and everything appeared to be working well, so I am reasonably confident that deploying this branch will not break GG. Could you please approve this PR at your convenience?

I will then get it deployed to the production lambda instance; and upgrade the production lambda instance node version to v18.

Please do not hesitate to reach me with any questions on this.

[Rovack]

I guess the only question is whether the SSTT - which is the same FE and BE as GoGetta, but some additional endpoints beyond GETs - also still works.

[jbeard4]

Will try to run this locally and get back to you.

[jbeard4]

SSTT LGTM:

@Rovack please approve at your convenience.

[Rovack]

Just to clarify: The screenshots seem to show 401 being returned for the requests (which I do remember being a problem with running the SSTT locally, unless some env var is passed or something...). Did you manage to get past this and see that data successfully updates? If not, maybe deploying and checking on the test environment would make it easier, as you suggested.

Of course, even if the SSTT breaks, it only affects internal users, so could be addressed if and when it happens. So, if it turns out testing it locally/in the test env is more trouble than it's worth, prod testing might not be out of the question. Whatever you think.