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.

Sign up for GitHub

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

Jump to bottom

Crash in ZoneStatusRetrieveAjaxView views from getlist() #1754

Merged
merged 3 commits into from
Aug 21, 2023
Merged

Crash in ZoneStatusRetrieveAjaxView views from getlist() #1754

merged 3 commits into from
Aug 21, 2023

Conversation

gromdimon
Copy link
Collaborator

Issue: #1752

Tasks:

  • Fix the bug
  • Test it

@gromdimon gromdimon linked an issue Aug 18, 2023 that may be closed by this pull request
Crash in ZoneStatusRetrieveAjaxView views from getlist() #1752
Closed
mikkonie
mikkonie requested changes Aug 18, 2023
View reviewed changes
Copy link
Contributor

@mikkonie mikkonie left a comment

Choose a reason for hiding this comment

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

Reviewed, found one bug and a style note.

landingzones/static/landingzones/js/landingzones.js Outdated Show resolved Hide resolved
landingzones/tests/test_views_ajax.py Outdated Show resolved Hide resolved
mikkonie
mikkonie requested changes Aug 18, 2023
View reviewed changes
Copy link
Contributor

@mikkonie mikkonie left a comment

Choose a reason for hiding this comment

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

This works now, good job! However, I would like one last bit of refactoring.

landingzones/static/landingzones/js/landingzones.js Outdated Show resolved Hide resolved
landingzones/views_ajax.py Outdated Show resolved Hide resolved
mikkonie
mikkonie approved these changes Aug 21, 2023
View reviewed changes
Copy link
Contributor

@mikkonie mikkonie left a comment

Choose a reason for hiding this comment

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

This still works as expected, which is great. The check in the ajax view I specifically asked for is missing, however it doesn't seem that is required with the current way we make requests to the Ajax view. Will merge.

@mikkonie mikkonie merged commit d93a00b into dev Aug 21, 2023
5 checks passed
@mikkonie mikkonie deleted the 1752-crash-in-zonestatusretrieveajaxview-views-from-getlist branch August 21, 2023 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikkonie mikkonie mikkonie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crash in ZoneStatusRetrieveAjaxView views from getlist()
2 participants
@gromdimon @mikkonie