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

Prototyping React #4

Draft
wants to merge 55 commits into
base: master
Choose a base branch
from
Draft

Prototyping React #4

wants to merge 55 commits into from

Conversation

vboctor
Copy link

@vboctor vboctor commented Apr 17, 2020

No description provided.

webdev77 and others added 18 commits April 14, 2020 11:01
Signed-off-by: Victor Boctor <[email protected]>
Signed-off-by: Victor Boctor <[email protected]>
It is much simpler to have the two arguments directly on the function.

Signed-off-by: Victor Boctor <[email protected]>
- Removed the ConfigService from JS
- Added calls to LocalizedStringsGetCommand() as sample and embedded the result html for JS.
- Added calls to ConfigsGetCommand() as samlpe and embedded the result in html for JS.

Signed-off-by: Victor Boctor <[email protected]>
export interface Relationship {
id: number;
type: RelationshipType;
issue: Issue;
Copy link
Author

Choose a reason for hiding this comment

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

@waltergar The relationship should use the proper Issue, but it can use IssueRelationshipRef or equivalent. This way the proper Issue model could enforce the right semantics, e.g. fields like description and project are mandatory. It also is strange to have an issue that contains a relationship that contains an issue.

</select>
&nbsp;
<input
type='text'
Copy link
Author

Choose a reason for hiding this comment

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

@waltergar Typing the issue id and then pressing Enter, no longer works.

Choose a reason for hiding this comment

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

@vboctor why not? I think it's working properly

<button
onClick={() => this.handleRelationshipAdd()}
className='btn btn-primary btn-sm btn-white btn-round'
>Add</button>
Copy link
Author

Choose a reason for hiding this comment

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

@waltergar Replace all hard-coded strings with localized one that are now available in the html div that I added.

* Default Bug dependent
* @global integer $g_default_bug_dependant
*/
$g_default_bug_dependant = BUG_DEPENDANT;
Copy link
Author

Choose a reason for hiding this comment

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

@waltergar There should be no need for us to add config options since we are trying to have parity with the existing system that didn't need such config.

vboctor added 2 commits April 18, 2020 14:23
Signed-off-by: Victor Boctor <[email protected]>
Supply the auto-complete candidates from the last visited issue list.
@vboctor vboctor force-pushed the react-relationships branch from 59cdef6 to 883968a Compare April 18, 2020 22:05
try {
const relationships = await this.Service.RelationshipAdd(this.state.reqRelTyp, parseInt(issueId));
this.setState({ relationships });
toast.success('Success!');
Copy link
Author

Choose a reason for hiding this comment

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

You can use localized string operation_successful for this.

@@ -114,8 +128,12 @@ export class IssueRelationships extends React.Component<Props, States> {
try {
const relationships = await this.Service.RelationshipDelete(relId);
this.setState({ relationships });
} catch (error) {

toast.success('Success!');
Copy link
Author

Choose a reason for hiding this comment

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

Use localized string operation_successful.

throw new Error(e.response.data.message);
else
throw new Error(e);
throw e;
Copy link
Author

Choose a reason for hiding this comment

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

Why catch the exception just to throw it again? I would just remove the try/catch from both this add/delete methods for relationships.

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