Skip to content

Universe campaign relations#77

Open
Skycrane112 wants to merge 8 commits intomainfrom
Universe-Campaign-Relations
Open

Universe campaign relations#77
Skycrane112 wants to merge 8 commits intomainfrom
Universe-Campaign-Relations

Conversation

@Skycrane112
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Owner

@BenDavidAaron BenDavidAaron left a comment

Choose a reason for hiding this comment

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

Left a few nits and some cleanup comments. Can be ready to merge in 15 mins of work or so IMO.

Comment thread src/crud/universe.rs
.await
}

pub async fn select_universes_by_user_id(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nit: This function's name is a little choppy. It fetches all the universes that a user is associated with, but we should consider breaking this up by permissions level in future work.
Can you think of a more descriptive name?
Not a blocker

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not so certain on this one so I left it as is. Would 'select_user_universes_by_user_id' be more appropriate?

Comment thread src/handlers/universe.rs Outdated
}
}

async fn get_universes(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nit: Move this function up a few lines so it's below get_universe, just for ease of browsing later.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Moved the function

Comment thread static/js/universe.js
throw error;
}
};

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What if a user want's to delete a universe or change the name of a universe?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Consider this function:

const updatePlayerCharacter = async (userId, characterId, characterData) => {

and this function
const deletePlayerCharacter = async (userId, characterId) => {

We want to have JS in the client that allows the browser to request updates or deletion of a universe. I think we should leave universe deletion out of the UI for now, since that's a mega destructive action.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Users can now update the name and description of their universes

Comment thread templates/base.html Outdated
<a href="/index.html">Home</a>
<a href="/campaigns.html">Campaigns</a>
<a href="characters.html">Characters</a>
<a href="/universes.html">Universes</a>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nit: Move this link ahead of Campaigns so it's first in the eye flow.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Moved the link

Comment thread templates/pages/universes.html Outdated
};

/*
const renderCampaignList = async () => {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you delete these commented out functions, it's sloppy to leave them in, especially in client side code that users can see 😉

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

👌

Comment thread templates/pages/universes.html Outdated


/*
const handleCampaignCreate = async (event) => {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This one too 🗑️

@BenDavidAaron BenDavidAaron changed the title *Draft - Universe campaign relations Universe campaign relations Nov 27, 2024
Comment thread static/js/universe.js
throw error;
}
};

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Consider this function:

const updatePlayerCharacter = async (userId, characterId, characterData) => {

and this function
const deletePlayerCharacter = async (userId, characterId) => {

We want to have JS in the client that allows the browser to request updates or deletion of a universe. I think we should leave universe deletion out of the UI for now, since that's a mega destructive action.

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.

2 participants