Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

InlinePanels with min_num are incorrectly ordered on first save #9391

Open
gasman opened this issue Oct 17, 2022 · 15 comments · May be fixed by #11889
Open

InlinePanels with min_num are incorrectly ordered on first save #9391

gasman opened this issue Oct 17, 2022 · 15 comments · May be fixed by #11889
Labels
component:Panels type:Bug

Comments

@gasman
Copy link
Collaborator

gasman commented Oct 17, 2022

Issue Summary

When an InlinePanel is defined with a min_num argument, an initial empty form is placed on the page. This form is given a blank Order value, which causes it to be ordered after any subsequently added forms when saved.

Steps to Reproduce

  1. Start a new project with wagtail start orderabletest
  2. Edit home/models.py to the following:
from django.db import models

from wagtail.models import Page, Orderable
from modelcluster.fields import ParentalKey
from wagtail.admin.panels import FieldPanel, InlinePanel


class HomePageTeasers(Orderable):
    page = ParentalKey("home.HomePage", related_name="teasers")
    teaser = models.CharField(max_length=255)

    panels = [
        FieldPanel("teaser"),
    ]


class HomePage(Page):
    content_panels = Page.content_panels + [
        InlinePanel("teasers", min_num=1),
    ]
  1. ./manage.py makemigrations / ./manage.py migrate / ./manage.py createsuperuser / ./manage.py runserver
  2. Log in to the admin and edit the homepage
  3. Enter "teaser 1" into the existing teaser field, then add a second teaser and enter "teaser 2" into it
  4. Save the page as draft

Expected behaviour: the form is reshown with "teaser 1" above "teaser 2"
Actual behaviour: "teaser 2" appears first

  • I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: yes

Technical details

  • Python version: 3.8.0
  • Django version: 4.1.2
  • Wagtail version: main (4.1a0)
  • Browser version: Chrome 105.0.5195.125
@gasman gasman added type:Bug component:Panels status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. labels Oct 17, 2022
@chosak chosak removed the status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. label Oct 17, 2022
@MaryAyobami
Copy link
Contributor

@gasman Are you working on this?
@thibaudcolas I'd love to be assigned to it

@thibaudcolas
Copy link
Member

@MaryAyobami we generally don’t assign issues to anyone directly except the core team. All contributors can choose to work on any issue they like. You’re very welcome to work on this, though if you’ve not done contributions to InlinePanel before this might be a tough one.

First step would be to check what in the code causes this initial form to be added to the formset with a blank order – and based on this, decide what to change.

@lb- lb- mentioned this issue Oct 17, 2022
Closed
@MaryAyobami
Copy link
Contributor

Alright I'd start working on it

@lb-
Copy link
Member

lb- commented Oct 23, 2022

@MaryAyobami how are you going with this one? Have you been able to reproduce the problem?

I'm happy to help with this as it would be good to see if we can fix this in time for 4.1 - feel free to reach out on Slack also.

@gasman
Copy link
Collaborator Author

gasman commented Oct 23, 2022

@lb- I don't think this would be a candidate for 4.1 - after feature freeze we only really want to apply bugfixes if they're regressions in that particular release (or serious enough that they'd be backported to other currently-supported releases).

@lb-
Copy link
Member

lb- commented Oct 24, 2022

no problems, thanks Matt

@gasman gasman mentioned this issue Dec 17, 2022
Closed
@Nazi-pikachu
Copy link
Contributor

Hi @gasman @lb- is this issue up for grab, i would like to work on this.

@gasman
Copy link
Collaborator Author

gasman commented Jan 11, 2023

@Nazi-pikachu Sure, go ahead!

@GeekGawd
Copy link
Contributor

Hi @gasman @lb- sorry LB, if my tag bothers you here as I have already reached out on Slack (as you offered some help in your previous comment).

My Findings are:

After inspecting request.POST

 "teasers-0-ORDER":[
      ""
   ],

the order being sent from frontend is an empty string

I am not that much knowledeable in html... I think the data is being sent from client/src/components/InlinePanel/index.js if I am not wrong.

However, console.log doesn't seem to be effective for me to debug, am I on the right path? How should I go about debugging the frontend. I tried using debugger; in javascript but that didn't seem to work.

@lb-
Copy link
Member

lb- commented Jan 24, 2023

@GeekGawd have you got your local developer set up working? You should be able to enter a debugger statement and then have it trigger when working locally.

I am not 100% sure on the underlying problem at the moment but sounds like you are heading in the right direction.

Another way to go about this is to try to reproduce the problem in the unit test file and then debug/resolve from there?

@GeekGawd
Copy link
Contributor

@lb- Yep i have got my local development setup up and running, the hot-reload works when I make changes to the wagtail project.

Well, I will try to get my hands dirty with html and js, and will reach out to you if I reach a dead end, Thanks!

@gasman
Copy link
Collaborator Author

gasman commented Jan 24, 2023

FWIW, I think this is probably a Django issue rather than an HTML / JS one - I'm pretty sure the ORDER field is blank in the initial form objects that get constructed on page creation, when we really want them to be pre-populated with their initial index numbers.

@GeekGawd
Copy link
Contributor

GeekGawd commented Feb 7, 2023

@lb- So, weeks into this, I think I am certain whats happening... I have noticed that Initial-Form is always 0 in wagtail however, this is wrong, because as per django, you can clearly that the forms order only gets pre filled during initial form creation.

The problem is when frontend first fires to make new Inline Panels, it is not passing INITIAL_FORMS and that is causing it to be 0, however in reality it should be min_num, I am unable to locate where in frontend should I make changes to pass INITIAL_FORMS

@lb-
Copy link
Member

lb- commented Feb 24, 2023

@GeekGawd sorry for the delayed response - your reasoning seems sound but I have not done a deep dive myself.

Do you have an idea about how we would fix this?

@GeekGawd
Copy link
Contributor

@lb- I can't confirm this but I think when the inline panels are initialized, the INITIAL_FORMS count in the form should be updated to min_num, I was having a hard time for weeks because I am weak in javascript and couldn't find the code where I can do this...

@ababic ababic mentioned this issue May 19, 2023
Merged
5 tasks
@lb- lb- mentioned this issue Feb 11, 2024
Open
@elhussienalmasri elhussienalmasri linked a pull request Apr 23, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component:Panels type:Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix the order of InlinePanel with min_num argument elhussienalmasri/wagtail
7 participants
@gasman @chosak @thibaudcolas @lb- @Nazi-pikachu @GeekGawd @MaryAyobami