Skip to content

Introduce utility method to look up drive props by drive name.#888

Merged
rdmark merged 3 commits intodevelopfrom
rdmark-prop-by-drive-name
Oct 6, 2022
Merged

Introduce utility method to look up drive props by drive name.#888
rdmark merged 3 commits intodevelopfrom
rdmark-prop-by-drive-name

Conversation

@rdmark
Copy link
Copy Markdown
Member

@rdmark rdmark commented Oct 4, 2022 •
edited
Loading

  • Introduce get_properties_by_drive_name() utility method. This looks up the drive props against the name of the drive. It also checks for duplication in the database.
  • Use it in web.py to reduce code duplication, and to remove client side parameters in drive.html (somewhat more secure)

@rdmark rdmark requested a review from nucleogenic October 4, 2022 05:19
@rdmark
Copy link
Copy Markdown
Member Author

rdmark commented Oct 4, 2022

drives

This change shaved almost 8k off the size of drives.html ;)

nucleogenic
nucleogenic approved these changes Oct 6, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@nucleogenic nucleogenic left a comment

Choose a reason for hiding this comment

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

It definitely makes sense to restrict the amount of data we receive from the client to the bare minimum - less to validate and sanitise. IMO, this is a good step in the right direction!

@nucleogenic
Copy link
Copy Markdown
Member

nucleogenic commented Oct 6, 2022

@rdmark Please can you check the tests are passing before merging. I've approved as I think the fixes might be in another one of your PRs already.

@rdmark
Copy link
Copy Markdown
Member Author

rdmark commented Oct 6, 2022

@rdmark Please can you check the tests are passing before merging. I've approved as I think the fixes might be in another one of your PRs already.

Sorry about that. My excuse for sometimes forgetting to run the tests sometimes is because I have to hack the working dir paths in api/conftest.py every time. ;)

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 6, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@rdmark rdmark merged commit 90ace5f into develop Oct 6, 2022
@rdmark rdmark deleted the rdmark-prop-by-drive-name branch October 6, 2022 21:04
rdmark added a commit that referenced this pull request Oct 7, 2022
@rdmark
Introduce utility method to look up drive props by drive name. (#888)
e3a4ef7 
* Introduce utility method to look up drive props by drive name.

* Make /drive/create endpoint fetch file ending from properties data structure; update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nucleogenic nucleogenic nucleogenic approved these changes

@akuker akuker Awaiting requested review from akuker akuker is a code owner

@erichelgeson erichelgeson Awaiting requested review from erichelgeson

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rdmark @nucleogenic