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: Actions from different views on same page #2008

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dosenpfand
Copy link
Contributor

Description

When having a page with different views e.g. a main and one or multiple related views with actions, they do not work as intended as all HTML <form>s will have the same ID action_form. This PR fixes the issue by appending the model view name to the ID.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

@Dosenpfand Dosenpfand changed the title fix: Support actions from different views on same page fix: Actions from different views on same page Mar 20, 2023
@dpgaspar dpgaspar self-requested a review May 12, 2023 09:55
@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Merging #2008 (6fc1cb8) into master (f591ee5) will increase coverage by 4.04%.
Report is 2 commits behind head on master.
The diff coverage is n/a.

❗ Current head 6fc1cb8 differs from pull request most recent head 3b91a7f. Consider uploading reports for the commit 3b91a7f to get more accurate results

@@            Coverage Diff             @@
##           master    #2008      +/-   ##
==========================================
+ Coverage   74.50%   78.54%   +4.04%     
==========================================
  Files          72       72              
  Lines        8970     8698     -272     
==========================================
+ Hits         6683     6832     +149     
+ Misses       2287     1866     -421     
Flag Coverage Δ
python 78.54% <ø> (+4.04%) ⬆️

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

see 39 files with indirect coverage changes

@Dosenpfand
Copy link
Contributor Author

Hi @dpgaspar, is there a chance to get this merged soonish?

@Dosenpfand
Copy link
Contributor Author

@dpgaspar , I applied the fix on the current tip of master. Please let me know if there is anything else I can do to get this merged. Thanks!

@DBREngineer
Copy link

@dpgaspar , It would be very nice to apply this patch.

Thanks

@ThomasP0815
Copy link
Contributor

@dpgaspar
I think this PR would make #2219 obsolete.
Is there a reason why this PR was never accepted ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants