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

WIP: Always use backend response for requests to source/:project_name #16955

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

krauselukas
Copy link
Contributor

@krauselukas krauselukas commented Oct 15, 2024

Right now we use the presence of the ?deleted=0 query parameter
as a workaround to force the request being passed directly to the
backend and returning results based on backend data over returning
results based on frontend database entries.

This workaround currently leads to inconsistant results being returned
by the API.

Instead of using this workaround we better should always
let the backend handle the request (where possible) and return
the data based on it's info.

Fixes #16911
Replaces #16928

So in short, after introducing this change we always deliver what ?delete=0 was doing before.

@krauselukas krauselukas added Frontend Things related to the OBS RoR app API Things regarding our API labels Oct 15, 2024
end

def render_project_issues
set_issues_defaults
render partial: 'source/project_issues', formats: [:xml]
end

def render_project_packages
@packages = params.key?(:expand) ? @project.expand_all_packages : @project.packages.pluck(:name)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the expand parameter can be handled by the backend as well

@krauselukas krauselukas changed the title Always use backend response for requests to source/:project_name WIP: Always use backend response for requests to source/:project_name Oct 15, 2024
@krauselukas
Copy link
Contributor Author

Set back to WIP since the minitests indicated some gaps...

# This implicitly also checks if the user can access the project (for hidden projects).
# We have to make sure to initialize the project already at this
# point, even we dont need the object in most cases because of that fact.
# TODO: Don't implicitly use the finder logic to authorize!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@krauselukas krauselukas force-pushed the fix/issue_16911 branch 3 times, most recently from 513e531 to cab76c6 Compare October 18, 2024 13:04
Right now we use the presence of the `?deleted=0` query parameter
as a workaround to force the request being passed directly to the
backend and returning results based on backend data over returning
results based on frontend database entries.

This workaround currently leads to inconsistant results being returned
by the API.

Instead of using this workaround we better should always
let the backend handle the request (where possible) and return
the data based on it's info.

Fixes openSUSE#16911
In this case we want the return to avoid double render errors.
After changing the `source/:project_name` to always pass the
request to the backend, the `count` attribute is not longer
part of the `directory` tag. This was only included by the frontend.
Since we now always get the info from the backend on
`source/:project_name` the multibuild flavors are included as well.
We have to skip those when querying for
`source/:project_name/:package_name/_meta` since multibuild flavors
dont have their own meta file.
After creating package along the test, the frontend knows
only about what exists in the fixtures (db) instead, but the backend still
has the created package
return
end

render_project_packages
raise InvalidParameterError, "'#{params[:view]}' is not a valid 'view' parameter value." unless params[:view].in?(%w[verboseproductlist productlist issues info])
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we show this message?

Comment on lines 37 to +39
packages.elements('entry') do |p|
# skip multibuild flavors
next if p['name'].include?(':')
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to use functional approaches when possible. It moves out the complexity from inside each iteration:

Suggested change
packages.elements('entry') do |p|
# skip multibuild flavors
next if p['name'].include?(':')
packages.elements('entry').reject {|p| p['name'].include?(':') }.each do |p|

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Things regarding our API Frontend Things related to the OBS RoR app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API inconsistency of the GET /source/{prj_name} route with deleted=0 parameter showing multibuild packages
3 participants