Skip to content

FieldChoie relation role#1693

Open
susanodd wants to merge 43 commits intomasterfrom
1688-add-new-items-to-relation_role_choices-or-make-relation_role_choices-dynamic
Open

FieldChoie relation role#1693
susanodd wants to merge 43 commits intomasterfrom
1688-add-new-items-to-relation_role_choices-or-make-relation_role_choices-dynamic

Conversation

@susanodd
Copy link
Collaborator

@susanodd susanodd commented Feb 9, 2026

A similar approach is being used to introduce the field choice field as was done with the original introduction of field choices

CODE IMPLEMENTATION

Details
  • migration to create field role_fk in Relation as a FieldChoiceForeignKey
  • population migration to store the constant choices into the new Field Choice RelationRole
  • data migration of the role CharField into the new FieldChoice field
  • retrieval methods combined into singular parameterised retrieval method
  • updated gloss relations template to make use of dictionary lookup of relation types instead of hard coding
  • CODE REVISION USING NEW FIELD TYPES, the original CharField has a character string as its domain, not an object, and the values now accessed using role_fk.name
  • error checking on "add_relation" to prevent duplicates (plus checks that the role type exists in RoleRelation of FieldChoice, this should not happen because it's a model field of the form)
  • CSV export and import Relations column revised to new type (checked on identical file being exported and imported without errors)
  • relations added to gloss revision history, additions to revision history also for CSV update
  • replaced redundant code with functions
  • reverse role now a method of FieldChoice as well as Relation
  • The usages of field role have been revised to use the new FieldChoice type
  • Feedback shows the changes to other relations (reverse relations) in the overview of changes to do for the CSV (shown prior to the user performing "do changes")
  • Tests on CSV update of relations, test on detection of erroneous input, and on creation of reverse relations

No deletion of relations performed during CSV update gloss.
[it's already implemented like this and it stays like this, the following example illustrates what happens.]

EXAMPLE

Details
  • row 1: gloss TEMPORARYTESTGLOSS_1 updates its relations column to: Synonym:TEMPORARYTESTGLOSS_2

  • row 2: gloss TEMPORARYTESTGLOSS_2 updates its relations column to: Hyponym:TEMPORARYTESTGLOSS_3

  1. when row 1 is processed, the state of gloss TEMPORARYTESTGLOSS_2 has been updated with the reverse Synonym relation to TEMPORARYTESTGLOSS_1

  2. when row 2 is processed, the initial state of gloss TEMPORARYTESTGLOSS_2 now has the new synonym in it, but in the CSV the "new" state should be: Hyponym:TEMPORARYTESTGLOSS_3

The code shows the "side effects" to the user now in the "changes" (where the "do changes" button is offered prior to doing the changes).

OPEN

  1. add method/model to set up whether a relation is symmetric, transitive, reversed with a different name in the other direction (this is partly hard-coded for different relation types) (OPTIONAL, see issue description)
  2. migration to remove the role CharField field and rename the new role_fk field to replace it

(2) must be done afterwards in a separate pull request

order as relation retrieval method, target gloss id, role machine value
Initial test on relations CSV. Data structures for passing state of relation updates to prevent conflicts (...)
@susanodd

This comment was marked as outdated.

of relations; update preview shows reverse relations in list of changes
@susanodd

This comment was marked as outdated.

@susanodd

This comment was marked as resolved.

@susanodd
Copy link
Collaborator Author

This branch has been merged with master to update all the packages. All the tests pass. signbank-susan is sync'd up to here.

@vanlummelhuizen
Copy link
Collaborator

@susanodd I still get an error:

screenshot image

@susanodd

This comment was marked as resolved.

@vanlummelhuizen
Copy link
Collaborator

Are all the name fields showing in your form? Does it not scroll down?

Nope

@susanodd
Copy link
Collaborator Author

susanodd commented Mar 20, 2026

@vanlummelhuizen it's fixed now!

(Not yet on the development server)

@vanlummelhuizen
Copy link
Collaborator

@susanodd Creating a new FieldChoice for RelationRole and it's reverse seems to work now. When I want to change one of them, something still does not work. First, not all language fields for the reverse are shown. Perhaps this is not a problem because a reverse relation is selected. Second, when I change one of the name fields (e.g. name_en) and save it, I get an error:

MultiValueDictKeyError at /admin/dictionary/fieldchoice/1504/change/

'reverse_identity'

...
image

@susanodd
Copy link
Collaborator Author

susanodd commented Mar 23, 2026

@vanlummelhuizen this should be working now.

I implemented lots of "context dependent" checks to try to only allow specific kinds of updates. (Fixed)

This is where your bug with the shown languages was being caused by:

MODELTRANSLATION_LANGUAGES = ['en','nl','zh-hans','ar','he']
# MODELTRANSLATION_LANGUAGES = ['en','nl','zh-hans']

To replicate your bug I had to switch these two (comment out the first one and uncomment the second). This causes the "database" fields for the translation languages to not match the "settings" for the translation languages.
This is actually an error if the database languages do not match the settings....

I fixed it now! So in the fields of the form it also uses the setting, not the database.

For really old field choices without all the languages filled in, it is required to fill in all the names in the translation languages (since the introduction of model translations instead of the old name_english). (Pages in the Admin are also like this, I had previously put in explicitly an empty space into About pages without any text in order to save the translations.)

Another weird thing, it's possible to create a field choice for fields that exist in the categories but are not used by Signbank global. (Because the Admin shows all the categories. Probably these need to somehow be blocked.)

Details This is a discussion of the "contextual constraints" on filling in the form. This should be working now.

The user is only able to update the name fields and the color.

For an already existing field of category relation role, it is not possible to edit the reverse relation.
This can only be done on creation.

If the user is not satisfied when the new role + reverse role, the user needs to delete these and create it again.

[GOAL OF CONSTRAINTS ON EDIT FORM -- WHAT IT OUGHT TO DO]

The constraints only only allow to create a relation role once. You can't edit the reverse relation on ones that already exist. This is to prevent altering existing data. (Like to prevent nonsense.)

The "reverse identity" is only used for creation. It's not a real field. It's meant so that the user either specifies an identity (self) reverse, or fills in the name fields. (It's not possible to choose the reverse from the pull down. There cannot be two reverse relations pointing to the same thing. That's why it's either or when making a new one. The new reverse fields should not already exist.)

It should kick the user back to the form and show what's wrong.

It's not possible to edit the reverse stuff for other kinds of fields. Those should not be editable.

@susanodd
Copy link
Collaborator Author

@vanlummelhuizen this should be working now. Just ignore the above discussion if it's working.

@vanlummelhuizen
Copy link
Collaborator

@susanodd You solved the error.

When testing, I came to the conclusion that a relation role and it's reverse relation can only be created in one go. So, if you first create a relation role that reverses to itself, you cannot later on create another reverse relation for it. That also means the field Reverse Relation does not do anything on the form to create a relation role. Perhaps we should make that clear to the user, e.g. by not showing the field on the create-form.

@susanodd
Copy link
Collaborator Author

susanodd commented Mar 24, 2026

@susanodd You solved the error.

When testing, I came to the conclusion that a relation role and it's reverse relation can only be created in one go. So, if you first create a relation role that reverses to itself, you cannot later on create another reverse relation for it. That also means the field Reverse Relation does not do anything on the form to create a relation role. Perhaps we should make that clear to the user, e.g. by not showing the field on the create-form.

That's a great suggestion. I've implemented that.

I'm testing the various combinations in the Admin interface now.

There are some quirks with it loading the correct combination of fields. It seems dependent on the control flow prior to the commands. Sometimes the reverse stuff that's supposed to be hidden is visible. But it disappears when the page is reloaded. I suspect there's another Django django method that's getting between. Like for some reason the "super" method is finding those reverse named fields, but they're not in the model and not in the form, they're dynamic. It seems maybe it's getting information from the "previous" creation of the form?

Here below, off-line I've put a print statement to see what's in fields, and it has them there from "super".

def get_fields(self, request, obj=None):
fields = super(FieldChoiceAdmin, self).get_fields(request, obj)
if obj and obj.machine_value is not None:
# the field choice already exists, only show the editable fields
return fields

This is what it is trying to debug it (but this should not need to be done):

    def get_fields(self, request, obj=None):
        fields = super(FieldChoiceAdmin, self).get_fields(request, obj)
        if obj is not None and obj.field != 'RelationRole':
            # the field choice already exists, only show the editable fields
            print('fields obj is defined: ', fields)
            fields = [f for f in fields if not f.startswith('reverse')]
            print('after removing: ', fields)
            return fields

I'm trying to remove the "reverse_name_" fields, because they end up in the form from super. Any ideas?
But if I reload the form, it fixes it. (The various functions aren't playing nice together...)

@susanodd
Copy link
Collaborator Author

@vanlummelhuizen aside from what I describe above with control flow, it's working now.

(My virtual environment got totally messed up after trying the branch from CoPilot, hence the delay.)

I can't solve the "presence or absence" of the reverse name fields. It's a bit inconsistent. Seems related to specific user control flow and to how you go back to "the listview of all the field choices" (via a button or going via the back button of the browser.)

I think it's working okay to allow merging.

@susanodd
Copy link
Collaborator Author

signbank-susan is sync'd up to here

@vanlummelhuizen
Copy link
Collaborator

@susanodd All is working correctly. Great! It would be nice if the reverse_name fields are not shown in the change form. First, because I get only two of five field and second, because they cannot be changedin that form anyway.

@susanodd
Copy link
Collaborator Author

@susanodd All is working correctly. Great! It would be nice if the reverse_name fields are not shown in the change form. First, because I get only two of five field and second, because they cannot be changedin that form anyway.

Okay, I'll try again. I need to go through the Django order of evaluation of the various Admin functions to figure out where it's doing that.

@susanodd
Copy link
Collaborator Author

susanodd commented Mar 27, 2026

@vanlummelhuizen @Woseseltops

Whatever changes have been recently done to the master are totally messing up PyCharm on this branch.

I merged master into this branch.

But PyCharm keeps "deleting everything" for some reason. It does not seem to recognise that this is the same project.

Did the modifications to the Pages cause some major code changes to the project structure?

FOUND IT

There is a NEW MIGRATION for FLAT PAGES

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add new items to RELATION_ROLE_CHOICES *OR* make RELATION_ROLE_CHOICES dynamic

4 participants