Skip to content

Feature: split V2 Item model into Item and MagicItem models#889

Open
calumbell wants to merge 4 commits intoopen5e:stagingfrom
calumbell:feature/magicitem-model
Open

Feature: split V2 Item model into Item and MagicItem models#889
calumbell wants to merge 4 commits intoopen5e:stagingfrom
calumbell:feature/magicitem-model

Conversation

@calumbell
Copy link
Contributor

Description

This PR takes the Item endpoint and splits it into Item (for mundane items) and MagicItem (for magic items)

This work was undertaken because of issues encountered while working on the implementation of crossreferences (PR #876). With the current setup of the API, it was not possible to create crossreferences to the /magicitems endpoint because it is an alias of the /items, and there is no distinct Magic Items table in the DB. This meant that the front-end would need to perform and additional fetch for each magic item crossreference on a page to put together the link, which would be very slow and inefficient.

Both Item and MagicItem inherit from the same abstract base class BaseItem, which stores all the fields ones would expect any item to have (the Item endpoint doesn't actually add any additional fields to the base-class). Therefore, for API consumers, any application that works with Item data (ie. an inventory management system) should be able to easily substitute a MagicItem in place of an Item because the former contains all fields defined in the later.

Note to reviewers

The large number of lines changed is due to our existing Item data getting split into mundane and magic items. You can see how this was achieved by checking out the split_magic_mundane_items_v2.py script in scripts/data_manipulation/converters/

Probably a better use of your time to focus on the changes made to V2 Models, Serializers and Views.

Other notes

While working on this PR I encountered a bug with Django getting V1 and V2 endpoints mixed up (more details #888). Instead of fixing this bug here, I have updated updated the V1 test_root approved file data to reflect the new shape of the API response, but that doesn't mean it is at a point where we wnt it to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant