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

Add IDs to common objects #592

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ subprocess = "0.1"
tempfile = "3.2.0"
termcolor = "1.1"
urlencoding = "2.1.0"
url = "2.2.2"
version-compare = "0.1.0"
webbrowser = "0.8.3"
is-terminal = "0.4.9"
Expand Down
1 change: 1 addition & 0 deletions src/database/parameter_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub struct ParameterDetails {
impl ParameterDetails {
pub fn get_property(&self, property_name: &str) -> String {
match property_name {
"id" => self.id.clone(),
"name" => self.key.clone(),
"value" => self.value.clone(),
"type" => self.param_type.clone(),
Expand Down
1 change: 1 addition & 0 deletions src/database/user_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub struct UserDetails {
impl UserDetails {
pub fn get_property(&self, property_name: &str) -> String {
match property_name {
"id" => self.id.clone(),
"name" => self.name.clone(),
"type" => self.account_type.clone(),
"email" => match self.account_type.as_str() {
Expand Down
15 changes: 12 additions & 3 deletions src/environments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ fn proc_env_list(
println!("{}", list.join("\n"));
} else {
let mut table = Table::new("environment");
let mut hdr = vec!["Name", "Parent", "Description"];
let mut hdr = vec!["ID", "Name", "Parent", "Description"];
if show_times {
hdr.push("Created At");
hdr.push("Modified At");
}
table.set_header(&hdr);
for entry in details {
let mut row = vec![entry.name, entry.parent_name, entry.description];
let mut row = vec![entry.id, entry.name, entry.parent_name, entry.description];
if show_times {
row.push(entry.created_at);
row.push(entry.modified_at);
Expand Down Expand Up @@ -219,7 +219,14 @@ fn proc_env_tag_list(
println!("{}", list.join("\n"))
} else {
let mut table = Table::new("environment-tags");
let hdr = vec!["Name", "Timestamp", "Description", "Immutable"];
let hdr = vec![
"ID",
"Name",
"Timestamp",
"Description",
"Immutable",
"Environment_ID",
];
// if show_usage {
// hdr.push("Last User");
// hdr.push("Last Time");
Expand All @@ -228,10 +235,12 @@ fn proc_env_tag_list(
table.set_header(&hdr);
for entry in tags {
let row = vec![
entry.id,
entry.name,
entry.timestamp,
entry.description,
entry.immutable.map(|i| i.to_string()).unwrap_or_default(),
env_id.clone(),
];
// if show_usage {
// row.push(entry.last_use_user);
Expand Down
16 changes: 9 additions & 7 deletions src/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ fn proc_param_list(
} else if show_rules {
// NOTE: do NOT worry about errors, since we're only concerned with params (not values)
let mut table = Table::new("parameter");
let mut hdr = vec!["Name", "Param Type", "Rule Type", "Constraint"];
let mut hdr = vec!["ID", "Name", "Param Type", "Rule Type", "Constraint"];
if show_times {
hdr.push("Created At");
hdr.push("Modified At");
Expand Down Expand Up @@ -685,16 +685,17 @@ fn proc_param_list(

// setup the table headers and properties
if show_external {
hdr = vec!["Name", "FQN", "JMES"];
properties = vec!["name", "fqn", "jmes-path"];
hdr = vec!["ID", "Name", "FQN", "JMES"];
properties = vec!["id", "name", "fqn", "jmes-path"];
} else if show_evaluated {
hdr = vec!["Name", "Value", "Raw"];
properties = vec!["name", "value", "raw"];
hdr = vec!["ID", "Name", "Value", "Raw"];
properties = vec!["id", "name", "value", "raw"];
} else if show_parents || show_children {
hdr = vec!["Name", "Value", "Project"];
properties = vec!["name", "value", "project-name"];
hdr = vec!["ID", "Name", "Value", "Project"];
properties = vec!["id", "name", "value", "project-name"];
} else {
hdr = vec![
"ID",
"Name",
"Value",
"Source",
Expand All @@ -705,6 +706,7 @@ fn proc_param_list(
"Description",
];
properties = vec![
"id",
"name",
"value",
"environment",
Expand Down
14 changes: 10 additions & 4 deletions src/projects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::cli::{
use crate::database::{OpenApiConfig, ProjectDetails, Projects};
use crate::table::Table;
use crate::utils::{
error_message, parse_key_value_pairs, user_confirm, warn_missing_subcommand, warning_message,
DEL_CONFIRM,
error_message, get_uuid_from_url, parse_key_value_pairs, user_confirm, warn_missing_subcommand,
warning_message, DEL_CONFIRM,
};
use clap::ArgMatches;
use color_eyre::eyre::Result;
Expand Down Expand Up @@ -60,14 +60,20 @@ fn proc_proj_list(
println!("{}", list.join("\n"));
} else {
let mut table = Table::new("project");
let mut hdr = vec!["Name", "Parent", "Description"];
let mut hdr = vec!["ID", "Name", "Parent", "Parent_ID", "Description"];
if show_times {
hdr.push("Created At");
hdr.push("Modified At");
}
table.set_header(&hdr);
for entry in details {
let mut row = vec![entry.name, entry.parent_name, entry.description];
let mut row = vec![
entry.id,
entry.name,
entry.parent_name,
get_uuid_from_url(&entry.parent_url.clone()),
entry.description,
];
if show_times {
row.push(entry.created_at);
row.push(entry.modified_at);
Expand Down
4 changes: 2 additions & 2 deletions src/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ fn proc_users_list(
.collect::<Vec<String>>();
println!("{}", list.join("\n"))
} else {
let mut hdr = vec!["Name", "Type", "Role", "Email", "Description"];
let mut properties = vec!["name", "type", "role", "email", "description"];
let mut hdr = vec!["ID", "Name", "Type", "Role", "Email", "Description"];
let mut properties = vec!["id", "name", "type", "role", "email", "description"];
if show_times {
hdr.push("Created At");
hdr.push("Modified At");
Expand Down
18 changes: 18 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::fmt::Formatter;
use std::io::{stdin, stdout, Write};
use std::str;
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
use url::Url;

// The `DEL_CONFIRM` is the default value for delete confirmation across different types
pub const DEL_CONFIRM: Option<bool> = Some(false);
Expand Down Expand Up @@ -214,6 +215,23 @@ pub fn parse_tag(input: Option<&str>) -> Option<String> {
}
}

pub fn get_uuid_from_url(url: &str) -> String {
if let Ok(url) = Url::parse(url) {
let path_segments: Vec<_> = url.path_segments().unwrap().collect();
if let Some(uuid_segment) = path_segments.get(3) {
if uuid_segment.len() == 36 {
Copy link
Member

Choose a reason for hiding this comment

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

Might be a little cleaner to define a constant here like UUID_LENGTH OR UUID_SEGMENT_LENGTH instead of the raw 36.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look!

I've had a chance to think about this a bit too, this is too specific to be in the utils file. If I tried using it for something other than projects it would fail or be inconsistent since we string ids and the ids, for example parameters, could be at another position in the vector.

I think having the parent project's id would be useful, but it may not be worth it unless you or someone else has some advice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, you are correct, that's a better way re: constant

uuid_segment.to_string()
} else {
"".to_string()
}
} else {
"".to_string()
}
} else {
"".to_string()
}
}

Copy link
Contributor

@erratic-pattern erratic-pattern Feb 9, 2024

Choose a reason for hiding this comment

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

Suggested change
pub fn get_uuid_from_url(url: &str) -> String {
if let Ok(url) = Url::parse(url) {
let path_segments: Vec<_> = url.path_segments().unwrap().collect();
if let Some(uuid_segment) = path_segments.get(3) {
if uuid_segment.len() == 36 {
uuid_segment.to_string()
} else {
"".to_string()
}
} else {
"".to_string()
}
} else {
"".to_string()
}
}
pub fn get_uuid_from_url(url: &str) -> Option<String> {
let url = Url::parse(url).ok()?;
let path_segments: Vec<_> = url.path_segments()?.collect();
let uuid_segment = path_segments.get(3)?;
if uuid_segment.len() == 36 {
Some(uuid_segment.to_string())
} else {
None
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! Thanks Adam, good to hear from you! Hope the new gig is going great!

I will surely do this. I went down this path at first, but couldn't reconcile the type I was sending into the caller (and Option return type) and got it to the point where "it works" in hopes that the review would shake out the fumbling-my-way-through-it approach! ;)

Also, I still feel it's not quite a useful utility function in that it's quite specific to the URL pattern "http:///<api_path>/<api_version>///" where the UUID has to be in the 3 position, but I also don't care to overcomplicate it, so moving the function into the projects file seems prudent, especially since that's all I intended for this "feature"?

/// Return the default value of a type according to the `Default` trait.
///
/// The type to return is inferred from context; this is equivalent to
Expand Down
Loading