4273 – Refactor API Seeds #1798
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
vasconsaurus
force-pushed
the
4273-refactor-api-seeds
branch
from
bae90ba to
24e4485
Compare
vasconsaurus
force-pushed
the
4273-refactor-api-seeds
branch
2 times, most recently
from
b2784c9 to
057c76e
Compare
vasconsaurus
force-pushed
the
4273-refactor-api-seeds
branch
from
b767bb8 to
9dff7fd
Compare
I was creating media and project media again, and I don't think that was necessary
This shouldn't create any issues, and will make creating objects easier
We were hoping to create everything when creating projects using nested team_attributes. Unfortunately we can't create everything nested at once, because: - team needs to exist for project media - team user needs to exist because of a callback Still, re-structuring things has helped: - making it easier to see what each object needs to be created and how they relate - making it more flexible to create how many users and full workspaces we need There is still a lot missing, but I think it makes sense to rethink how we create things, and this seems like a good direction.
I think this can be a private method of this class, also changed the method name from medias_params to get_medias_params, though I'm not too sure about it
Could use attr_reader or access the instance variable using @ But I liked this post: https://ivoanjo.me/blog/2017/09/20/why-i-always-use-attr_reader-to-access-instance-variables/ And confirmed with Caio this was a good way to go.
I think the setup shouldn't get this info and print it it should just get it, and we do whatever we wnat with it Nothing serious, but it was bugging me
We need to call get_medias_params each time, because the urls need to be different for all users
When setting the params to create project medias we are now also setting the claim_description_attributes. To do this I had to add the association and accepts_nested_attributes_for in project_media.rb
The way we dealt with this before, is: we would rescue link errors, print a message to the user and move forward with our script creating other media. I'm not sure if that will work while crating with nested attributes. Even if I rescue it seems to fail adding the project_medias, though it does create the project. So for now, I'm printing an useful message and not showing the user information if it errors. But I'll look further into this.
So we can create fact checks when creating claims. Since we don't want to create fact checks for all claims, I added reject_if, when we send a blank summary it does not create a fact check.
just to make what is being passed clear
Because the interface looks different, we create 2 feeds: - feed_1 shares Published Fact-Checks - feed_2 shares both Published Fact-Checks and Media, Claim & Requests, and has clusters
I think we can make less of each type of item: - we will still have a bunch in total - it will be faster (one hopes)
vasconsaurus
force-pushed
the
4273-refactor-api-seeds
branch
from
9dff7fd to
094166e
Compare
The other type of medias can remain as constants: - makes it easier if we only want to create items from CLAIMS_PARAMS instead of calling medias_params - the links need to be created each time for each user so that we get different timestamps I also added a items_total method, so it can check that for that, instead of hardcoding a number
The minimun amount of items we need to create is 8, so each type of media should create that (so we can run only one of them if needed) for relationships and tiplines I tried to make it more dynamic: - so it doesn't break with the minimum amount - if we create everything, it creates more of those
I'm trying to play around with ways to make this faster - using constants - creating the links only for the main user I don't like this yet, but the time did drop from 10 minutes to 6, so I'm leaving it like this for now I added a conditional in cluster_teams to make sure it does not break when we don't create invited teams
vasconsaurus
changed the title
vasconsaurus
requested review from
caiosba,
melsawy and
DGaffney
as code owners
|
FYI @vasconsaurus I pushed up a merge with develop to try to clear up those bundle message we chatted about |
|
love it @vasconsaurus : |
|
Code Climate has analyzed commit ceb4e7c and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (100% is the threshold). This pull request will bring the total coverage in the repository to 100.0% (0.0% change). View more on Code Climate. |
|
Just so I understand the expectations of the data @vasconsaurus 
|
|
That's correct, @brianfleming - for now those are just random numbers because we currently only list the cards. In the next sprint, when we have the item page, the numbers will reflect real data. |
caiosba
approved these changes
Feb 29, 2024
There was a problem hiding this comment.
Thanks @vasconsaurus , the code is way more clear, great refactoring!
We don't need to keep this file, right? -> db/seeds_backup.rb? Can we just delete it?
yes! will delete it and merge this |
vasconsaurus
force-pushed
the
4273-refactor-api-seeds
branch
from
ceb4e7c to
f681d03
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First
Apologies as this got way too big, I probably should have broken it up by action (creating relationships, tipline requests etc)
Description
Our old seeds script was built to create items for one user, once the need to have more users appeared there wasn't an easy way to update it.
This file separates everything into two objects: setup and populated_workspaces.
[1] Setup
[2] Populated Workspaces
References: 4273, 4174
How has this been tested?
By running the seeds to create a new user and add a new user.
Notes
Setup.users.all_users.merge!(invited_users))Setup.teams.all_teams.merge!(invited_teams))MEDIAS_PARAMS, comment out type of medias you do not want to createprojects_params_main_user_aandproject_params_invited_users)Sidekiq::Queue.all.map(&:clear)I tried to summarize what I thought was important, if there are any questions please let me know.
I know it's a lot, so I'm sure there is a bunch of confusing stuff in there.