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

feat: introduce variations to fail to add users (WPB-2299) #15497

Merged
merged 1 commit into from
Jul 24, 2023

Conversation

V-Gira
Copy link
Contributor

@V-Gira V-Gira commented Jul 24, 2023

StoryWPB-2299 [Web] Errors when adding users from non-fully connected graph

feature

  • add an errorMesssage prop to the failedToAddUser component

@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #15497 (6225bd1) into dev (a1eb192) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev   #15497   +/-   ##
=======================================
  Coverage   43.43%   43.44%           
=======================================
  Files         649      649           
  Lines       22076    22092   +16     
  Branches     5048     5052    +4     
=======================================
+ Hits         9588     9597    +9     
- Misses      11255    11263    +8     
+ Partials     1233     1232    -1     

@V-Gira V-Gira marked this pull request as ready for review July 24, 2023 07:52
@V-Gira V-Gira requested review from a team and otto-the-bot as code owners July 24, 2023 07:52
Comment on lines 143 to -125
{isOpen && (
<>
{total <= 1 && (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

condition isOpen is true when total > 1 so this will never happen

@@ -49,6 +55,7 @@ const config = Config.getConfig();
const FailedToAddUsersMessage: React.FC<FailedToAddUsersMessageProps> = ({
isMessageFocused,
message,
errorMessageType = ErrorMessageType.offlineBackEnd,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The different error messages are not part of the scope of the September release.
The only error message displayed stays the same as offline back-end

Comment on lines +111 to +132
{total <= 1 && (
<p data-uie-name="1-user-not-added-details" data-uie-value={users[0].id}>
<span
css={warning}
dangerouslySetInnerHTML={{
__html: t(`failedToAddParticipantSingularOfflineBackEnd`, {
name: users[0].name(),
domain: users[0].domain,
}),
}}
/>
{learnMore}
</p>
)}
{total > 1 && (
<p
css={warning}
dangerouslySetInnerHTML={{
__html: t(`failedToAddParticipantsPlural`, {total: total.toString()}),
}}
/>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be simplified to just total <= 1 ? () : (), no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could, I kept it the way @thisisamir98 did it.
Probably easier to read?

Copy link
Contributor

Choose a reason for hiding this comment

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

The result will be the same, I'm fine with it

@V-Gira V-Gira merged commit 8b2fd10 into dev Jul 24, 2023
13 checks passed
@V-Gira V-Gira deleted the virgile/failure-to-add-participant branch July 24, 2023 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants