#206 Create Table: stack_element_type (merge conflicts)

[AzaniaBG]

Fixes #206

What changes did you make?

• created a StackElementType model in Django and test with the following Data Fields:
  • name: CharField type
  • description: TextField type
  • did not add id field because this is the uuid (added this under the read_only_fields for Serializer's Meta class definition
• registered the model
• created a Serializer and View Set
• wrote an API end point and test and registered the endpoint
• documented the API endpoint (added the extend_schema_view decorator and doc strings where applicable)
• merged PermissionType table changes from the HfLA main branch

Why did you make the changes (we will use this info to test)?

• Completed tasks related to creating the Stack Element Type table, which are set forth in the Action Items section of issue 206
• I added the verbose name in the model's Meta class because I thought it might be helpful
• I merged conflicts because the main branch had new PRs approved/merged prior to this issue being completed

[fyliu]

For some reason Github is showing the wrong changes in the migrations directory. I tried the admin site and it works fine.

The verbose_name_plural field is mainly for correcting cases where adding an "s" at the end of the model name doesn't make it plural. It's not really useful here.

The serializers.py class needs some more fields and read_only_fields. Use the other models as examples.

[AzaniaBG]

For some reason Github is showing the wrong changes in the migrations directory. I tried the admin site and it works fine.

The verbose_name_plural field is mainly for correcting cases where adding an "s" at the end of the model name doesn't make it plural. It's not really useful here.

The serializers.py class needs some more fields and read_only_fields. Use the other models as examples.

@fyliu ~

1. I'm not sure what changes you are requesting regarding the verbose_name_plural field. Do you want that removed?

2. I'm not sure if you need a fix because GitHub is showing the wrong changes in the migrations directory.

3. Regarding the serializer, I thought we did not need to add uuid to the field (only to the read_only_fields). What additional fields need to be added to fields besides "name" and "description"?

   • I added "created_at" and "updated_at" to read_only_fields.`
   • Please let me know what other fields need to be added to the fields property.

[fyliu]

@AzaniaBG

I just realized I've been looking at the wrong branch. Your PR is actually in the "merge-upstream-..." branch. I'll try everything again.

Yes, I think you should remove the verbose_name_plural since it doesn't really do anything special.

Nothing to be done about github's display. It's just a comment. I can see the actual changes just fine when I fetch the code.

Yes, add the timestamps to read_only. And also add uuid to the fields. The fields list is what becomes available through the API. So if a client needs to update/delete a stack_element_type, they would need to get the list of them, find the uuid of the one they're interested in, and use the uuid in the next request.

[fyliu]

@AzaniaBG I found a major problem with the migrations. The old migration files are being replaced by this PR, which is not good.

In the first screenshot below, we're looking at what's in the repo at the main branch (1). Notice the filenames for migrations 14-17 (2).

The 2nd screenshot below is a view in this PR branch (1). I put a (2) at the same migrations here.

I stacked the screenshots to better show the differences and outlined 14-17 in both branches.

My point is that the migrations in main should NOT be overwritten. Another developer should be able to call migrate at any point in the main branch and get a consistent database. That means you should add new migrations at the end.

I realize this is something that came out of a merge conflict. I thought that git merge and git rebase could yield the same result, (update: I think it can still be done both ways, with more care for git merge) but in the scenario of needing to preserve main migrations, git rebase is the better workflow to use. I'll have to mention this in the docs and make it the recommended way.

This situation is a compound issue from several things.

1. You started your work from a commit that's not in main, because it's not been merged yet. There's a chance that that commit is not what actually ends up in main, like what happened here.

2. 0014 was deleted because it was the last migration and it seemed easier and cleaner to do that. But maybe we should rethink that. Because another PR got merged before the delete language one, it actually added a separate migration to delete language.

3. It just happens that 0015 and 0016 are the same in both branches, so git merge didn't really complain about them.

4. There are 2 migration files in the same commit before the merge, which means the django-linear-migrations tool can't handle the conflict automatically. But with the issue API Endpoints for CivicTechJobs #2 above, and it not been merged into the repo yet, it wouldn't have done anything anyway.

I think you should squash the 2 commits from the current issue and combine the migration files, either manually or delete and regenerate a new one. Then switch to main and cherry-pick the one commit. It will cause a migration conflict and you'll need to rename your generated migration file to be the next higher number from what's already there. It's also possible to keep the 2 separate commits, but there's more steps to do that for not much gain.

[AzaniaBG]

Update: I'm working on the changes Fang requested.
Availability:
11/3: 10 AM - 12 PM
11/6: 10 AM - 12 PM

[AzaniaBG]

@fyliu ~ I re-requested review

[fyliu]

@AzaniaBG I'm reviewing this but I don't see the re-request review. Is there something you need to click or did you already click it? Anyway, I'm only wondering what happened.

[AzaniaBG]

@fyliu I thought I clicked the re-review button. I had to force push the changes.

[fyliu]

Thanks for making the changes! I checked that it runs and the API works.