Add check to resource_modification for orphaned langs/cats

[mike-lloyd03]

I'm unable to test anything without an Angolia key but I put this together just to see if I'm on the right track.

Fixes #344

[aaron-junot]

You can actually sign up for a free trial of Algolia here https://www.algolia.com/users/sign_up

That will give you an API key that you can use for testing this kind of thing if you want. But we should be able to make sure most of it works how we need it to without the algolia part, and then we can test out the algolia stuff in staging and if there's a problem just fix it before we go to prod

[aaron-junot]

I don't like a SQL SELECT being inside a for loop at all. We should be able to select everything we need just once and work with that, right?

It would require more memory to store all the Resources in an array and then search for the specific ones we need.. not sure if the trade-off of time for more memory is the right answer either...

I think optimizing this will require more research and thinking it through. And perhaps it's not a huge deal because this route isn't used very often, maybe we can worry about optimizing it later.

If we do decide to kick that can down the road, we should leave a comment about it's possible performance issues so when someone (us?) is trying to hunt down the cause of a performance problem later, it's easier to find.

[mike-lloyd03]

I'd rather get it right now. I can change this to query the database once.

[aaron-junot]

I took a look at the failing tests. I have a suspicion about the problem with a couple of them, they look legit. If you want to jump on a call to troubleshoot those, just let me know

[mike-lloyd03]

@aaron-suarez I am trying another approach at this PR which doesn't have a query in a for loop. I've only implemented the check on the Category modification just to see what you think so far. If you think it looks good. I'll go ahead and apply the logic to the Language modification.

[aaron-junot]

This is not the category name as a string. This is a Category object. This is the definition of a Category object:

resources_api/app/models.py

Lines 106 to 130 in a78426a

	class Category(db.Model):
	id = db.Column(db.Integer, primary_key=True)
	name = db.Column(db.String, nullable=False)
	@property
	def serialize(self):
	"""Return object data in easily serializeable format"""
	return {
	'id': self.id,
	'name': self.name,
	}
	def key(self):
	return self.name
	def __eq__(self, other):
	if isinstance(other, Category):
	return self.name == other.name
	return False
	def __hash__(self):
	return hash(self.name)
	def __repr__(self):
	return f"<Category {self.name}>"

To get the string representation of the category, you'll want resource.category.name

[aaron-junot]

Does this actually need to be a SQL query? Can we not just loop through the array of strings resource_categories and look for old_category and if it's not in there, we know that old_category is orphaned?

I haven't thought through it a whole lot, so if the answer is we do indeed need a SQL query then I'm willing to hear about why. But at first glance it looks like we don't

[mike-lloyd03]

Since old_category is a Category object, we can just call old_category.delete() on it if it's orphaned right? In that case, we wouldn't need the query you highlighted above. I can just tweak the following if statement and be done with it. I'll have another commit tonight.

[aaron-junot]

I think you delete with db.session.delete(old_category)

[mike-lloyd03]

Thanks for saying "I think" so I don't look as dumb for saying that.

[mike-lloyd03]

Okay I think we're good to go now. I cleaned up the logic and implemented similar logic for the Languages portion.

[aaron-junot]

Can we get some unit tests around it? We want to make sure that an unused Category is deleted, an unused language is deleted, and that no categories or languages that are still in use are deleted

[mike-lloyd03]

@aaron-suarez Okay I cherry-picked from @king-11 's tests branch but CircleCI is still failing. I'll take a look later tonight.

[mike-lloyd03]

Okay I fixed the botched cherry-pick with more commits than necessary. But hopefully this is the end.

[aaron-junot]

Love it! Thanks for your help, y'all!