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.

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

feat: rearranging standard fields in customize form #19822

Merged
merged 50 commits into from Jun 8, 2023

Conversation

dj12djdjs
Copy link
Collaborator

@dj12djdjs dj12djdjs commented Jan 28, 2023

continuation of: #19452

Customize Form now supports reordering all fields (including "standard" ones) 🎉

Implementation

A new property setter called field_order has been introduced. When meta.sort_fields is called following is done:

  • if field order doesn't exist, existing algorithm applies

If field order exists:

  • field order is "cleaned" for dead fields
  • if it matches size of fields, new order is immediately updated (best case scenario)

In case new fields are present in doctype, but aren't in cleaned field order:

  • if standard fields at the top of the DocType's fields table are absent in the field order, a list is created of such fields.
  • if there are atleast some standard fields in field order, the above list is prepended to field order, else, the above list becomes field order (essentially field order gets invalidated)
  • remaining fields are inserted using following logic:
    • insert_after for custom fields (same as before)
    • computed insert_after for standard fields (this is currently done based on the DocType's fields table)

Additional implementation (if reviewer requests it)

  • Delete invalid field_order (if no standard fields found) by enqueue-ing a DB call
  • Update field order (when re-computed) by enqueue-ing a DB call
  • Notify system managers if field order needs update. The "computed insert after" logic isn't 100% accurate so it can be beneficial if user reviews the order and manually fixes as needed (needs above update feature as well - to prevent duplicate notifications)

cc: @ankush @surajshetty3416

TODO (after initial review)

  • tests for "computed" logic
  • support for form builder
  • documentation

To be backported together with #20168

@codecov
Copy link

codecov bot commented Jan 28, 2023

Codecov Report

Merging #19822 (5ef66f3) into develop (7042dca) will increase coverage by 0.12%.
The diff coverage is 54.54%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #19822      +/-   ##
===========================================
+ Coverage    63.87%   64.00%   +0.12%     
===========================================
  Files          761      759       -2     
  Lines        68844    68755      -89     
  Branches      6224     6200      -24     
===========================================
+ Hits         43977    44007      +30     
+ Misses       21320    21256      -64     
+ Partials      3547     3492      -55     
Flag Coverage Δ
server 68.91% <54.90%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@dj12djdjs

This comment was marked as outdated.

@pmjd-code
Copy link

I can offer to help with documentation if needed, would be great to see this implemented.

@dj12djdjs
Copy link
Collaborator Author

I can offer to help with documentation if needed, would be great to see this implemented.

@pmjd-code Thank you, I think we better wait until code changes are approved before writing documentation.

@dj12djdjs dj12djdjs marked this pull request as ready for review February 4, 2023 13:12
@dj12djdjs dj12djdjs requested a review from a team as a code owner February 4, 2023 13:12
@dj12djdjs dj12djdjs requested review from phot0n and removed request for a team February 4, 2023 13:12
@sagarvora

This comment was marked as outdated.

@sagarvora

This comment was marked as outdated.

@frappe frappe deleted a comment from stale bot Apr 19, 2023
@stale stale bot added the inactive label Apr 26, 2023
@frappe frappe deleted a comment from stale bot Apr 26, 2023
@stale stale bot removed the inactive label Apr 26, 2023
@pmjd-code
Copy link

The Form Builder is great and this feature would really help in customising apps.
I know there is a lot going on and limited resources but does anyone know if this feature likely to make it soon or is it more likely months away?

@stale stale bot added the inactive label May 11, 2023
@frappe frappe deleted a comment from stale bot May 12, 2023
@stale stale bot removed the inactive label May 12, 2023
@sagarvora
Copy link
Collaborator

I will complete the remaining TODOs over the weekend and merge this.

If anyone has inputs on the additional implementation points above, they'd be greatly appreciated.

@sagarvora
Copy link
Collaborator

I haven't been able to get this done. Some other projects that are more critical are taking longer. I don't have a new ETA as of now.

@pmjd-code
Copy link

@sagarvora I understand. Thank you for the update and all of the hard work from you and @dj12djdjs on this.

If it is only the 4 files listed on thir PR needed to implement, I will probably try and add them in my own install so I can customise my installation of ERPnext more easily.

Thanks

@stale stale bot added the inactive label May 25, 2023
@frappe frappe deleted a comment from stale bot May 25, 2023
@stale stale bot removed the inactive label May 25, 2023
frappe/model/meta.py Show resolved Hide resolved
@ankush ankush merged commit 19d211f into frappe:develop Jun 8, 2023
19 of 22 checks passed
@ankush
Copy link
Member

ankush commented Jun 8, 2023

@dj12djdjs @sagarvora this feels okay for first cut. Whatever the caveats maybe, all those exist for current code too.

Few things we can add maybe is:

@sagarvora
Copy link
Collaborator

support for form builder added via #21297

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 24, 2023
@dj12djdjs dj12djdjs deleted the customize_form branch July 24, 2023 17:49
@sagarvora sagarvora added backport version-14-hotfix backport to version 14 and removed defer backport Backports for some PR are deferred for a week or two to test them properly before releasing labels Aug 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants