Skip to content

Conversation

@FiloSpaTeam
Copy link
Contributor

Hey Friends (yikes! like the CONTRIBUTING.md :) )
actually fixed this one #15403;

What's wrong with the old code?
Using the method Token.token_type, the class itself changes and the form_with(@token) use the new form class to name the params, so the require(:token) will throw an error.
But the problems is not always available. You need to make a wrong search on project name or package name.

So setting the type with the write_attribute(:type) and removing the token_type allow to set the :type back and render correctly the :new partial.
The fix correlated regards the default value for the :type in the radio_buttons, was hard typed with service.

@github-actions github-actions bot added the Frontend Things related to the OBS RoR app label Jan 16, 2024
@codecov
Copy link

codecov bot commented Jan 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (edd2237) 86.25% compared to head (ef82a8c) 86.25%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #15489   +/-   ##
=======================================
  Coverage   86.25%   86.25%           
=======================================
  Files         789      789           
  Lines       25832    25833    +1     
=======================================
+ Hits        22281    22282    +1     
  Misses       3551     3551           

@krauselukas
Copy link
Contributor

@FiloSpaTeam thanks for your contribution! I will have a look on the changes

@krauselukas
Copy link
Contributor

krauselukas commented Jan 18, 2024

@FiloSpaTeam Hey, I had a look. The changes looking good to me, took me a bit to understand the issue itself. Could you do me a favor and just add the reasoning for the changes (basically what you wrote in the PR description) into the commit message as well. Always good to have it tracked in Git itself. And maybe remove the the issue number from the commit title and just mention it in the description. Thanks!

@FiloSpaTeam
Copy link
Contributor Author

@FiloSpaTeam Hey, I had a look. The changes looking good to me, took me a bit to understand the issue itself. Could you do me a favor and just add the reasoning for the changes (basically what you wrote in the PR description) into the commit message as well. Always good to have it tracked in Git itself. And maybe remove the the issue number from the commit title and just mention it in the description. Thanks!

No problem. I ll do tomorrow morning :)

@FiloSpaTeam FiloSpaTeam changed the title fix #15403; change how to set params type to new token in set_package change how to set params type to new token in set_package Jan 18, 2024
Using the method `Token.token_type` for set instance var `@token` after a wrongly
search on package name or project name, will change the class type
of the @token, that set up a wrong `form_with` that returns a wrong
params name after submit. So i changed the method above with
`write_attribute` to set the type.
@krauselukas krauselukas merged commit 5dd07db into openSUSE:master Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Frontend Things related to the OBS RoR app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants