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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: dont delete customizations when doctypes are deleted #17009

Merged
merged 4 commits into from
May 30, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented May 27, 2022

If someone deletes doctype and restores it back all customization are
lost, there's no "real" reason to delete all these customization. They
are only ever active if the doctype is being used.

Explanations:

  • Custom field: is used by meta when doctype meta is requested, if meta
    isn't requested then the custom field is effectively inactive.
  • Client script: loaded by meta when doctype is requested by the desk. So
    inactive in a deleted state.
  • Property setter: loaded by meta, so inactive when doctype isn't
    present.
  • Report: will break doctype isn't present, but the user should delete them
    manually to avoid loss of "scripts" or anything special they might
    have done. Also, report's doctype don't 100% indicate that it's based
    solely on that doctype.

More unrelated fixes:

IDK any other error that occurs during routine usage with stale doctype/modules. If you find any point it out and we will handle it better 馃槄 basically:

  • Opening the stale doctype page should throw error
  • anything else should not be affected by non-exisitng doctypes.

@ankush ankush marked this pull request as ready for review May 27, 2022 14:38
@codecov
Copy link

codecov bot commented May 27, 2022

Codecov Report

Merging #17009 (82cc983) into develop (d193ea2) will decrease coverage by 1.70%.
The diff coverage is 64.51%.

@@             Coverage Diff             @@
##           develop   #17009      +/-   ##
===========================================
- Coverage    58.36%   56.66%   -1.71%     
===========================================
  Files          770      770              
  Lines        68879    68883       +4     
  Branches      6005     6005              
===========================================
- Hits         40200    39030    -1170     
- Misses       25070    26244    +1174     
  Partials      3609     3609              
Flag Coverage 螖
server 61.06% <64.51%> (-2.70%) 猬囷笍

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

@ankush ankush requested a review from a team May 29, 2022 07:38
If someone deletes doctype and restores it back all customization are
lost, there's no "real" reason to delete all these customization. They
are only ever active if the doctype is being used.

Explanations:

- Custom field: is used by meta when doctype meta is requested, if meta
  isn't requested then custom field is effectively inactive.
- Client script: loaded by meta when doctype is requested by desk. So
  inactive in deleted state.
- Property setter: loaded by meta, so inactive when doctype isn't
  present.
- Report: will break doctype isn't present, but user should delete them
  manually to avoid loss of "scripts" or anything special they might
  have done. Also report's doctype don't 100% indicate that it's based
  solely on that doctype.
Whole lot of unnecessary complexity, closure, multiple function calls,
comprehensions all that can be replaced with single `get_all`
This is done during app install stage and causes unnecessary failures
where stale doctypes are left in db.
@ankush ankush merged commit 75b7a82 into frappe:develop May 30, 2022
@ankush ankush deleted the delete_doctype_changes branch May 30, 2022 15:30
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant