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

Don’t return story translations if user already translating/reviewing for language #229

Merged
merged 1 commit into from
Dec 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 19 additions & 4 deletions backend/python/app/graphql/queries/story_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ def resolve_story_by_id(root, info, id):
return services["story"].get_story(id)


def resolve_stories_available_for_translation(root, info, language, level):
return services["story"].get_stories_available_for_translation(language, level)
def resolve_stories_available_for_translation(root, info, language, level, user_id):
target_user_id = _select_user_id_for_available_translations_query(user_id)

return services["story"].get_stories_available_for_translation(
language, level, user_id=target_user_id
)


def resolve_story_translations_by_user(
Expand All @@ -29,9 +33,13 @@ def resolve_story_translation_by_id(root, info, id):
return services["story"].get_story_translation(id)


def resolve_story_translations_available_for_review(root, info, language, level):
def resolve_story_translations_available_for_review(
root, info, language, level, user_id=None
):
target_user_id = _select_user_id_for_available_translations_query(user_id)

return services["story"].get_story_translations_available_for_review(
language, level
language, level, user_id=target_user_id
)


Expand All @@ -51,3 +59,10 @@ def resolve_story_translation_tests(root, info, language, level, stage, story_ti
return services["story"].get_story_translation_tests(
user, language, level, stage, story_title
)


def _select_user_id_for_available_translations_query(user_id):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Putting this here because moving it to story_service means adding the user service to the story service. Also, this sticks to the principle of keeping authentication out of the story services. lmk if you can think of a better name tho LOL

Copy link
Collaborator

Choose a reason for hiding this comment

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

what do you think about something like target_user_id instead of searching_user_id? i'm not sure if it's more readable or not

and consequently something like _target_user_id_for_available_translations_query for the helper

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the verb "select" makes more sense than "target" for the function name, but I will swap searching_user_id for target_user_id !

# if caller is admin, use param use_id. Else, use caller id
calling_user_id = get_user_id_from_request()
isAdmin = services["user"].get_user_by_id(calling_user_id).role == "Admin"
return user_id if isAdmin else calling_user_id
16 changes: 12 additions & 4 deletions backend/python/app/graphql/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,13 @@ class Query(graphene.ObjectType):
graphene.List(StoryTranslationResponseDTO),
language=graphene.String(required=True),
level=graphene.Int(required=True),
user_id=graphene.ID(),
)
stories_available_for_translation = graphene.Field(
graphene.List(StoryResponseDTO),
language=graphene.String(required=True),
level=graphene.Int(required=True),
user_id=graphene.ID(),
)

def resolve_comments_by_story_translation(
Expand Down Expand Up @@ -171,8 +173,12 @@ def resolve_user_by_id(root, info, id):
def resolve_user_by_email(root, info, email):
return resolve_user_by_email(root, info, email)

def resolve_stories_available_for_translation(root, info, language, level):
return resolve_stories_available_for_translation(root, info, language, level)
def resolve_stories_available_for_translation(
root, info, language, level, user_id=None
):
return resolve_stories_available_for_translation(
root, info, language, level, user_id
)

def resolve_story_translations_by_user(
root, info, user_id, is_translator=None, language=None, level=None
Expand Down Expand Up @@ -205,9 +211,11 @@ def resolve_story_translation_tests(
def resolve_story_translation_by_id(root, info, id):
return resolve_story_translation_by_id(root, info, id)

def resolve_story_translations_available_for_review(root, info, language, level):
def resolve_story_translations_available_for_review(
root, info, language, level, user_id=None
):
return resolve_story_translations_available_for_review(
root, info, language, level
root, info, language, level, user_id
)


Expand Down
75 changes: 50 additions & 25 deletions backend/python/app/services/implementations/story_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,13 @@ def create_story(self, story, content):

return new_story

def get_stories_available_for_translation(self, language, level):
stories = (
Story.query.filter(Story.level == level)
.filter(~Story.translated_languages.contains(language))
.all()
)
return [story.to_dict(include_relationships=True) for story in stories]

def create_translation(self, translation):
try:
new_story_translation = StoryTranslation(**translation.__dict__)
story_translations_translating = (
self._get_story_translations_user_translating(
new_story_translation.translator_id
)
self._get_story_translations_user_translating_query(
new_story_translation.translator_id, isTranslator=True
).all()
)
languages_currently_translating = self._get_story_translation_languages(
story_translations_translating
Expand Down Expand Up @@ -173,7 +165,9 @@ def create_translation_test(self, user_id, level, language):
raise error
return new_story_translation

def get_story_translations_by_user(self, user_id, is_translator, language, level):
def get_story_translations_by_user(
self, user_id, is_translator=None, language=None, level=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.

made this change to make the function header more readable

):
if is_translator is None:
role_filter = (StoryTranslation.translator_id == user_id) | (
StoryTranslation.reviewer_id == user_id
Expand Down Expand Up @@ -430,8 +424,10 @@ def get_story_translation(self, id):
raise error

def assign_user_as_reviewer(self, user, story_translation):
story_translations_reviewing = self._get_story_translations_user_reviewing(
user.id
story_translations_reviewing = (
self._get_story_translations_user_translating_query(
user.id, isTranslator=False
).all()
)
languages_currently_reviewing = self._get_story_translation_languages(
story_translations_reviewing
Expand Down Expand Up @@ -608,8 +604,40 @@ def update_story_translation_stage(self, story_translation_data, user_id):
self.logger.error(error)
raise error

def get_story_translations_available_for_review(self, language, level):
def get_stories_available_for_translation(self, language, level, user_id):
try:
ongoing_translations = (
self._get_story_translations_user_translating_query(
user_id, isTranslator=True
)
.filter(StoryTranslation.language == language)
.all()
)
if len(ongoing_translations) > 0:
return []

stories = (
Story.query.filter(Story.level == level)
.filter(~Story.translated_languages.contains(language))
Copy link
Collaborator

Choose a reason for hiding this comment

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

just a q: what does the ~Story... syntax mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

negation! this reads as return story translations where translated_languages does not contain language

.all()
)
return [story.to_dict(include_relationships=True) for story in stories]
except Exception as error:
self.logger.error(str(error))
raise error

def get_story_translations_available_for_review(self, language, level, user_id):
try:
ongoing_translations = (
self._get_story_translations_user_translating_query(
user_id, isTranslator=False
)
.filter(StoryTranslation.language == language)
.all()
)
if len(ongoing_translations) > 0:
return []

stories = (
Story.query.join(
StoryTranslation, Story.id == StoryTranslation.story_id
Expand Down Expand Up @@ -848,18 +876,15 @@ def _get_num_approved_lines(self, translation_contents):

return count

def _get_story_translations_user_translating(self, user_id):
return (
StoryTranslation.query.filter(StoryTranslation.translator_id == user_id)
.filter(StoryTranslation.stage != "PUBLISH")
.all()
)

def _get_story_translations_user_reviewing(self, user_id):
def _get_story_translations_user_translating_query(self, user_id, isTranslator):
return (
StoryTranslation.query.filter(StoryTranslation.reviewer_id == user_id)
StoryTranslation.query.filter(StoryTranslation.is_test == False)
.filter(
StoryTranslation.translator_id == user_id
if isTranslator
else StoryTranslation.reviewer_id == user_id
)
.filter(StoryTranslation.stage != "PUBLISH")
.all()
)

def _get_story_translation_languages(self, story_translations):
Expand Down
17 changes: 15 additions & 2 deletions backend/python/app/services/interfaces/story_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,24 @@ def update_story_translation_stage(self, story_translation_data):
pass

@abstractmethod
def get_story_translations_available_for_review(self, language, level):
def get_stories_available_for_translation(self, language, level, user_id):
"""
Return a list of stories available to be reviewed by user
Return a list of stories available to be translated by user
:param level: level of user
:param language: language being searched for
:param user_id: user_id looking for stories to translate
:return: list of StoryDTO's
:rtype: list of StoryDTO's
"""
pass

@abstractmethod
def get_story_translations_available_for_review(self, language, level, user_id):
"""
Return a list of story translations available to be reviewed by user
:param level: level of user
:param language: language being searched for
:param user_id: user_id looking for stories to translate
:return: list of StoryTranslationResponseDTO's
:rtype: list of StoryTranslationResponseDTO's
"""
Expand Down
11 changes: 7 additions & 4 deletions frontend/src/APIClients/queries/StoryQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ export const buildHomePageStoriesQuery = (

export const buildAssignStoryQuery = (
isTranslator: boolean,
language: string | null,
level: number | null,
language: string,
level: number,
userId: number,
): QueryInformation => {
const result = isTranslator
? {
Expand All @@ -190,7 +191,8 @@ export const buildAssignStoryQuery = (
query StoriesAvailableForTranslation {
storiesAvailableForTranslation(
language: "${language}",
level: ${level}
level: ${level},
userId: ${userId}
) {
storyId: id
${STORY_FIELDS}
Expand All @@ -204,7 +206,8 @@ export const buildAssignStoryQuery = (
query StoriesAvailableForTranslation {
storyTranslationsAvailableForReview(
language: "${language}",
level: ${level}
level: ${level},
userId: ${userId}
) {
storyId
storyTranslationId: id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ const AssignedStoryTranslationsTable = ({
</Table>
{assignStory && (
<AssignStoryModal
userId={userId}
isOpen={assignStory}
onClose={closeAssignStoryModal}
onAssignStory={(story) => {
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/components/utils/AssignStoryModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export type StoryToAssign = {
};

export type AssignStoryModalProps = {
userId: number;
isOpen: boolean;
onClose: () => void;
onAssignStory: (story: StoryToAssign) => void;
Expand All @@ -44,6 +45,7 @@ export type AssignStoryModalProps = {
};

const AssignStoryModal = ({
userId,
isOpen,
onClose,
onAssignStory,
Expand All @@ -63,7 +65,12 @@ const AssignStoryModal = ({
null,
);

const query = buildAssignStoryQuery(role === "Translator", language, level);
const query = buildAssignStoryQuery(
role === "Translator",
language!,
level!,
userId,
);

useQuery(query.string, {
fetchPolicy: "cache-and-network",
Expand Down