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

fix(web_form): check properties for title field as well #23342

Conversation

akhilnarang
Copy link
Member

Without this, setting the show_title_field_in_link on sites without developer mode on seems useless, as it doesn't change anything

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
@akhilnarang akhilnarang requested review from a team and shariquerik and removed request for a team November 22, 2023 06:25
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Nov 22, 2023
@akhilnarang akhilnarang added backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15 labels Nov 22, 2023
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Merging #23342 (67b74cd) into develop (28d05c4) will increase coverage by 0.01%.
Report is 10 commits behind head on develop.
The diff coverage is 81.81%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #23342      +/-   ##
===========================================
+ Coverage    62.24%   62.26%   +0.01%     
===========================================
  Files          771      771              
  Lines        73891    74028     +137     
  Branches      6342     6344       +2     
===========================================
+ Hits         45996    46096     +100     
- Misses       24286    24342      +56     
+ Partials      3609     3590      -19     
Flag Coverage Δ
server 70.72% <81.81%> (+<0.01%) ⬆️

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

@shariquerik shariquerik merged commit 375ec5e into frappe:develop Nov 22, 2023
24 of 25 checks passed
@akhilnarang akhilnarang deleted the webform-check-title-link-from-properties branch November 22, 2023 07:18
akhilnarang added a commit that referenced this pull request Nov 22, 2023
…-23342

fix(web_form): check properties for title field as well (backport #23342)
akhilnarang added a commit that referenced this pull request Nov 22, 2023
…-23342

fix(web_form): check properties for title field as well (backport #23342)
@@ -692,6 +692,13 @@ def get_link_options(web_form_name, doctype, allow_read_on_all_link_options=Fals
if value and int(value) == 1:
show_title_field_in_link = True

if not title_field:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akhilnarang this is needlessly complicated!

Just use frappe.get_meta(doctype) which is mix of doctype + property setter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will open a PR to simplify

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
add-test-cases Add test case to validate fix or enhancement backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants