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 backwards relation references in room search #2595

Merged

Conversation

johannaengland
Copy link
Contributor

17ef4e0 broke things by adding a related name that was used otherwise (netboxes) and me changing location to child_locations, where it was actually incorrect, which resulted in the room search not working.

Luckily I caught this before it was included in a release.

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #2595 (c47e62b) into master (4672022) will decrease coverage by 0.16%.
The diff coverage is 0.00%.

❗ Current head c47e62b differs from pull request most recent head 50de7f0. Consider uploading reports for the commit 50de7f0 to get more accurate results

@@            Coverage Diff             @@
##           master    #2595      +/-   ##
==========================================
- Coverage   54.20%   54.05%   -0.16%     
==========================================
  Files         558      558              
  Lines       40634    40602      -32     
==========================================
- Hits        22026    21946      -80     
- Misses      18608    18656      +48     
Impacted Files Coverage Δ
python/nav/web/info/room/views.py 30.45% <0.00%> (ø)

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Mar 17, 2023

Test results

     12 files       12 suites   11m 27s ⏱️
3 238 tests 3 142 ✔️   96 💤 0
9 189 runs  8 901 ✔️ 288 💤 0

Results for commit 50de7f0.

♻️ This comment has been updated with latest results.

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

The code changes are good, but I'm not sure I see the point of the changelog entry...

CHANGELOG.md Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Mar 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

Since what this fixes was never released I am guessing what @lunkwill42 means is that the changelog is not necessary at all. Or did you mean something else?

@lunkwill42
Copy link
Member

Since what this fixes was never released I am guessing what @lunkwill42 means is that the changelog is not necessary at all. Or did you mean something else?

I mean exactly what it says in the ensuing discussion under the "Outdated" review item, @hmpf :)

17ef4e0 broke things by adding a related name that was used otherwise
@johannaengland johannaengland force-pushed the bugfix/room-search-related-names branch from c47e62b to 50de7f0 Compare May 25, 2023 06:22
@sonarcloud
Copy link

sonarcloud bot commented May 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johannaengland johannaengland merged commit b7f24ec into Uninett:master May 25, 2023
11 of 13 checks passed
@johannaengland johannaengland deleted the bugfix/room-search-related-names branch May 25, 2023 06:31
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

3 participants