Skip to content

fixing branding test #31

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fixing branding test #31

wants to merge 2 commits into from

Conversation

BMayhew
Copy link
Contributor

@BMayhew BMayhew commented Jan 14, 2025

Summary by CodeRabbit

  • Bug Fix: Updated logo URL in branding to dynamically fetch from Env.URL for improved branding consistency.
  • Test: Added assertion for response status code 202 in PUT requests test for branding, ensuring accurate API responses.

Remember, bugs are like ants in your code - they may be small, but they can cause a big picnic! 🐜🧺

Copy link

github-actions bot commented Jan 14, 2025

Image description CodeRabbit

Walkthrough

The changes focus on enhancing branding functionality by updating the logo URL dynamically and adding an assertion for a response status code in a PUT request test. These modifications aim to improve the application's branding features and ensure proper handling of HTTP requests.

Changes

File Summary
lib/helpers/branding.ts Added import statement for Env module, updated logoUrl to use dynamic URL from Env.URL.
tests/branding/branding.spec.ts Modified branding/ PUT requests test with an assertion for response status code 202 after a PUT request.

In the world of code, changes unfold,
The rabbit hops, the story is told.
Branding shines, tests are bold,
With every commit, excellence is strolled. 🐇🌟


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between ba5fe0b and d360bec commits.
Files selected (2)
  • lib/helpers/branding.ts (1)
  • tests/branding/branding.spec.ts (1)
Review comments generated (5)
  • Review: 5
  • LGTM: 0

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

@@ -1,10 +1,12 @@
import Env from "./env";

Choose a reason for hiding this comment

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

Image description CodeRabbit

Bruh, what's good with this import statement change? You swapped out the direct URL for a dynamic one from Env.URL. Make sure this change doesn't mess up the URL resolution logic. 🤔

export const defaultBranding = {
name: "Shady Meadows B&B",
map: {
latitude: 52.6351204,
longitude: 1.2733774,
},
logoUrl: "https://automationintesting.online/images/rbp-logo.jpg",
logoUrl: `${Env.URL}/images/room2.jpg`,

Choose a reason for hiding this comment

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

Image description CodeRabbit

Yo, fam! The logoUrl change looks lit, but ensure that Env.URL resolves to the correct base URL. Keep an eye on this one! 🔍

export const defaultBranding = {
name: "Shady Meadows B&B",
map: {
latitude: 52.6351204,
longitude: 1.2733774,
},
logoUrl: "https://automationintesting.online/images/rbp-logo.jpg",
logoUrl: `${Env.URL}/images/room2.jpg`,
description:
"Welcome to Shady Meadows, a delightful Bed & Breakfast nestled in the hills on Newingtonfordburyshire. A place so beautiful you will never want to leave. All our rooms have comfortable beds and we provide breakfast from the locally sourced supermarket. It is a delightful place.",

Choose a reason for hiding this comment

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

Image description CodeRabbit

This description block seems chill, no issues here. 🌟

Comment on lines +1 to 12
import Env from "./env";

export const defaultBranding = {
name: "Shady Meadows B&B",
map: {
latitude: 52.6351204,
longitude: 1.2733774,
},
logoUrl: "https://automationintesting.online/images/rbp-logo.jpg",
logoUrl: `${Env.URL}/images/room2.jpg`,
description:
"Welcome to Shady Meadows, a delightful Bed & Breakfast nestled in the hills on Newingtonfordburyshire. A place so beautiful you will never want to leave. All our rooms have comfortable beds and we provide breakfast from the locally sourced supermarket. It is a delightful place.",
contact: {

Choose a reason for hiding this comment

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

Image description CodeRabbit

Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [13-13]

No changes detected in this line range.

Keep it sharp, developer! Remember, details matter when it comes to code changes. Don't let those bugs sneak in like ninjas in the night! 💥🐰

Comment on lines +38 to +42
const response = await request.put("branding/", {
headers: headers,
data: defaultBody,
});
expect(response.status()).toBe(202);

Choose a reason for hiding this comment

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

Image description CodeRabbit

Bruh, we got a spicy issue here! 🌶️ Looks like you misspelled status as a function in expect(response.status()).toBe(202);. It should be expect(response.status).toBe(202);. Gotta keep those syntax errors in check, fam!

- expect(response.status()).toBe(202);
+ expect(response.status).toBe(202);

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

Successfully merging this pull request may close these issues.

1 participant