Feature: split V2 Item model into Item and MagicItem models#889
Open
calumbell wants to merge 4 commits intoopen5e:stagingfrom
Open
Feature: split V2 Item model into Item and MagicItem models#889calumbell wants to merge 4 commits intoopen5e:stagingfrom
calumbell wants to merge 4 commits intoopen5e:stagingfrom
Conversation
This file contains hidden or 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
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.
Description
This PR takes the
Itemendpoint and splits it intoItem(for mundane items) andMagicItem(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
/magicitemsendpoint 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
ItemandMagicIteminherit from the same abstract base classBaseItem, which stores all the fields ones would expect any item to have (theItemendpoint doesn't actually add any additional fields to the base-class). Therefore, for API consumers, any application that works withItemdata (ie. an inventory management system) should be able to easily substitute aMagicItemin place of anItembecause 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.pyscript inscripts/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_rootapproved 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.